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.

Sign up for GitHub

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

[Usage] Fix flaky UI Counters test #100979

Merged
merged 11 commits into from
Jun 3, 2021
Merged

[Usage] Fix flaky UI Counters test #100979

Changes from 4 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
dbe0bbb
increase to delay to 8 seconds
Bamieh May 31, 2021
822180d
[TEMP] ONLY UI COUNTERS TEST FILE
Bamieh Jun 1, 2021
64c029b
Update index.ts
Bamieh Jun 1, 2021
9dda838
Merge branch 'master' into usage/fix_ui_counters_test
kibanamachine Jun 1, 2021
b4e6f6f
Merge branch 'master' of github.com:elastic/kibana into usage/fix_ui_…
Bamieh Jun 2, 2021
229aa3e
Merge branch 'master' of github.com:elastic/kibana into usage/fix_ui_…
Bamieh Jun 3, 2021
651101e
retry assertion function
Bamieh Jun 3, 2021
04b583c
Merge branch 'usage/fix_ui_counters_test' of github.com:Bamieh/kibana…
Bamieh Jun 3, 2021
a8f8599
self codereview
Bamieh Jun 3, 2021
2e8d126
revert and use retry common util
Bamieh Jun 3, 2021
225c783
uncomment test files
Bamieh Jun 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion test/api_integration/apis/ui_counters/ui_counters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export default function ({ getService }: FtrProviderContext) {
.expect(200);

// wait for SO to index data into ES
await new Promise((res) => setTimeout(res, 5 * 1000));
await new Promise((res) => setTimeout(res, 8 * 1000));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we retry requesting /api/saved_objects/_find?type=usage-counters until it's succeeded instead of relying on a manual delay?

Copy link
Member Author

Choose a reason for hiding this comment

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

That is doable although the delay pattern is all across kibana tests (200+ places)

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, but it doesn't mean it's a good thing.

  • it leads to false positives due to changes in Kibana internals (what you are fixing right now)
  • it introduces unnecessary delays in the execution flow. Maybe the condition is true after 2sec?
  • it's not clear how to pick a correct value, devs tend to copy-paste them

Copy link
Member Author

Choose a reason for hiding this comment

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

I completely agree. What i meant is that we might need to resolve it across all tests rather than just this one test. I've created a helper util (with tests) to retry assertions. We can move it outside the tests/.../telemetry folder in a separate PR later on. Let me know what you think.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can move it outside the tests/.../telemetry folder in a separate PR later on.

ok then. Let's create an issue for Core-related tests.

};

const getCounterById = (
Expand Down