Skip to content

[Flaky Test] Fixes Space Navigation Race Condition#147193

Merged
jeramysoucy merged 3 commits intoelastic:mainfrom
jeramysoucy:fixes-flaky-spaces-nav-test
Dec 13, 2022
Merged

[Flaky Test] Fixes Space Navigation Race Condition#147193
jeramysoucy merged 3 commits intoelastic:mainfrom
jeramysoucy:fixes-flaky-spaces-nav-test

Conversation

@jeramysoucy
Copy link
Contributor

@jeramysoucy jeramysoucy commented Dec 7, 2022

Resolves #142155

Flakiness appears to be due to a race condition of clicking on a space item in the menu before the item is responsive to interaction. This PR attempts to confirm the space item click by checking the menu is no longer present, before moving on the verify the new space in the URL.

Flaky Test Runner Results

@jeramysoucy jeramysoucy added Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// test-failure-flaky labels Dec 7, 2022
@jeramysoucy jeramysoucy marked this pull request as ready for review December 8, 2022 18:56
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-security (Team:Security)

@jeramysoucy jeramysoucy added the release_note:skip Skip the PR/issue when compiling release notes label Dec 8, 2022
@jeramysoucy jeramysoucy requested a review from a team December 8, 2022 18:57
@kibana-ci
Copy link

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id before after diff
osquery 1 2 +1

ESLint disabled line counts

id before after diff
enterpriseSearch 19 21 +2
fleet 60 66 +6
osquery 109 115 +6
securitySolution 445 451 +6
total +20

Total ESLint disabled count

id before after diff
enterpriseSearch 20 22 +2
fleet 69 75 +6
osquery 110 117 +7
securitySolution 521 527 +6
total +21

History

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

Copy link
Contributor

@thomheymann thomheymann left a comment

Choose a reason for hiding this comment

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

LGTM, one question below

@@ -205,6 +205,7 @@ export class SpaceSelectorPageObject extends FtrService {
this.log.info(`SpaceSelectorPage:goToSpecificSpace(${spaceId})`);
await this.testSubjects.click(`${spaceId}-selectableSpaceItem`);
await this.common.sleep(1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need this artificial timeout now that we're checking for the menu to be closed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was inclined to leave it in because there seems to sometimes be a delay between the click and the closing of the menu/redirect. Without it, it may be possible for another race condition resulting in the space item not even being there on the next retry.
If this doesn't solve the problem we were seeing, I think I will refactor the test to be as deterministic as possible, in which case we can take out the delay.

@jeramysoucy
Copy link
Contributor Author

Ran the Flaky TR on test/server_integration/http/platform/config.status.ts as well, due to CI result.

@jeramysoucy jeramysoucy merged commit 13c5ab6 into elastic:main Dec 13, 2022
@kibanamachine kibanamachine added v8.7.0 backport:skip This PR does not require backporting labels Dec 13, 2022
@jeramysoucy jeramysoucy deleted the fixes-flaky-spaces-nav-test branch February 2, 2023 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Security Platform Security: Auth, Users, Roles, Spaces, Audit Logging, etc t// test-failure-flaky v8.7.0

Projects

None yet

5 participants