Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Dataset quality] fixing flaky test for dataset details summary size #194683

Conversation

yngrdyn
Copy link
Contributor

@yngrdyn yngrdyn commented Oct 2, 2024

Closes #194575.

Flaky test runner here

@yngrdyn yngrdyn requested a review from a team as a code owner October 2, 2024 11:07
@yngrdyn yngrdyn added backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-logs Observability Logs User Experience Team labels Oct 2, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-logs-team (Team:obs-ux-logs)

@yngrdyn yngrdyn force-pushed the 194575-serverless-observability-ui-dataset-quality-flyout-overview-summary-panel-should-show-summary-kpis branch from 0744b39 to 743cec2 Compare October 2, 2024 11:10
…uality-flyout-overview-summary-panel-should-show-summary-kpis
@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

✅ unchanged

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

Flaky Test Runner Stats

🎉 All tests passed! - kibana-flaky-test-suite-runner#7072

[✅] x-pack/test_serverless/functional/test_suites/observability/config.ts: 50/50 tests passed.

see run history

expect(parseInt(size, 10)).to.be.greaterThan(0);
// metering stats API is cached for 30seconds, waiting for the exact value is not optimal in this case
// rather we can just check if any value is present
expect(size).to.be.ok();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note

The .ok method is not recommended and asserts for truthiness, not existence, so it should fail too when the size is 0.
If we cannot rely on concrete criteria for asserting the value of size, it makes it more difficult to test this part as we cannot reliably know if we get the cached value.
I cannot suggest a better alternative, unfortunately, just raising that this assertion might trigger the same issue as before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it should fail too when the size is 0.

size is being returned as an string. As long as something is returned in the UI we should be fine.

we cannot reliably know if we get the cached value

This is the main problem that's why I'm proposing not to care about the value at least from a UI perspective and just assert we have a value there. For the accuracy of the value I prefer we rely on the API tests. wdyt?

@yngrdyn yngrdyn merged commit 036f35e into elastic:main Oct 4, 2024
21 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11178774963

@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Oct 4, 2024
… size (#194683) (#194927)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Dataset quality] fixing flaky test for dataset details summary size
(#194683)](#194683)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Yngrid
Coello","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-04T11:02:34Z","message":"[Dataset
quality] fixing flaky test for dataset details summary size
(#194683)\n\nCloses
https://github.com/elastic/kibana/issues/194575.\r\n\r\nFlaky test
runner\r\n[here](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7072)","sha":"036f35e3a6fc8acff52d06b45b21e4a795d002f1","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","v9.0.0","backport:prev-minor","Team:obs-ux-logs"],"title":"[Dataset
quality] fixing flaky test for dataset details summary
size","number":194683,"url":"https://github.com/elastic/kibana/pull/194683","mergeCommit":{"message":"[Dataset
quality] fixing flaky test for dataset details summary size
(#194683)\n\nCloses
https://github.com/elastic/kibana/issues/194575.\r\n\r\nFlaky test
runner\r\n[here](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7072)","sha":"036f35e3a6fc8acff52d06b45b21e4a795d002f1"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/194683","number":194683,"mergeCommit":{"message":"[Dataset
quality] fixing flaky test for dataset details summary size
(#194683)\n\nCloses
https://github.com/elastic/kibana/issues/194575.\r\n\r\nFlaky test
runner\r\n[here](https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7072)","sha":"036f35e3a6fc8acff52d06b45b21e4a795d002f1"}}]}]
BACKPORT-->

Co-authored-by: Yngrid Coello <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:skip Skip the PR/issue when compiling release notes Team:obs-ux-logs Observability Logs User Experience Team v8.16.0 v9.0.0
Projects
None yet
5 participants