SO Tagging: fix flaky test and re-enable it #82930

pgayvallet · 2020-11-09T09:51:33Z

Summary

mshustov · 2020-11-09T12:12:08Z

x-pack/test/saved_object_tagging/functional/tests/visualize_integration.ts

        await PageObjects.visualize.gotoVisualizationLandingPage();
+        await PageObjects.header.waitUntilLoadingHasFinished();


Suggestion: There could be a lot of reasons for waiting for a DOM element to be rendered. What if we increase timeout in selectFilterTags method instead?

TBH after 100 runs against my local I couldn't reproduce the flaky failure even once. The failure message indicates that for some reason, 1. the tag filter was not applied (Visualization 2 (tag-2) is present in the table) and 2. the new vis was not created (My new markdown viz is not), and I don't really how we could come as far as the last assertion in that state without failing before in the test. So I kinda did the best I could with additional synchronizations / waiting steps to see if that would resolves it on CI (renamed the PR accordingly).

I can do what you suggest though

@pgayvallet Have you tried to leverage https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/build
to run a flaky test on CI?

I did not. Can this be ran against a PR branch?

you can configure it

First run had a failure on another test - https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/978/testReport/junit/X-Pack%20Saved%20Object%20Tagging%20Functional%20Tests/x-pack_test_saved_object_tagging_functional_tests_dashboard_integration%C2%B7ts/Kibana_Pipeline___agent_2___saved_objects_tagging___functional_tests_dashboard_integration_creating_allows_to_create_a_tag_from_the_tag_selector/

but the error message was a little more explicit on that one. trying something else by waiting for the table to load

https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/986/

https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/989/

https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/996/

https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/997/

https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1002/

…-vis-flacky-ftr-1

pgayvallet · 2020-11-12T12:35:29Z

To reviewers: sorry, please don't mind this PR. Had to edit the test file ci groups to avoid the flaky test runner to run all the group. You can safely unsubscribe.

pheyos

ML changes LGTM

Edit:

To reviewers: sorry, please don't mind this PR. Had to edit the test file ci groups to avoid the flaky test runner to run all the group. You can safely unsubscribe.

Noticed this one too late 😅

…-vis-flacky-ftr-1

This reverts commit db86c3b

pgayvallet · 2020-11-16T13:43:26Z

Ran 3*200 using the flaky test runner, without failure. Seems that the additional table await is a correct fix.

https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/996/
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/997/
https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1002/

mshustov · 2020-11-17T12:52:53Z

x-pack/test/saved_object_tagging/functional/tests/visualize_integration.ts

@@ -133,7 +139,10 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
        expect(await tagModal.isOpened()).to.be(false);

        await testSubjects.click('confirmSaveSavedObjectButton');
+        await PageObjects.common.waitForSaveModalToClose();


do we still need it?

mshustov · 2020-11-17T12:59:02Z

x-pack/test/saved_object_tagging/functional/tests/visualize_integration.ts

        await PageObjects.visualize.gotoVisualizationLandingPage();
+        await listingTable.waitUntilTableIsLoaded();


I still don't understand whether we can get rid of this instruction in the test suit. As suggested in #82930 (comment)
IMO, It's okay in the preparatory phase but hard to debug in the test suite, so people start adding them all over the places.

There were some rare race conditions where the elements we are manipulating in the next instructions of the test were already removed from the DOM, because we retrieved them before the table was loaded (which cause a rerender, causing some existing dom elements to be removed from the document). This additional check is meant to fix the race by waiting for the next refresh

…-vis-flacky-ftr-1

kibanamachine · 2020-11-18T15:26:19Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 61980e9

Metrics [docs]

✅ unchanged

History

💚 Build #88499 succeeded 5861e50
💚 Build #87973 succeeded 5e9a93c
💚 Build #87899 succeeded c77f036
💚 Build #87831 succeeded 2909b19
💔 Build #87828 failed 46da2af

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

* fix flaky test and re-enable it * wait for table to load before to perform operations * move everything out of ciGroup2 for flaky test runner * add debug block for flaky runner * use correct vis name * remove test sync * Revert "move everything out of ciGroup2 for flaky test runner" This reverts commit db86c3b

…o-node-details * 'master' of github.com:elastic/kibana: SO Tagging: fix flaky test and re-enable it (elastic#82930) [Metrics UI] Converting legend key to optional (elastic#83495)

…t/saved_object_tagging/functional/tests/maps_integration·ts - saved objects tagging (#149356) Fixes #89073 and #106547 Flaky test runner https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1785 Issue with flaky maps_integration.ts is that there was not await when filtering maps list by tags. This resulted in list page refreshing after `const links = await this.find.allByCssSelector('.euiTableRow-isSelectable .euiLink');` and then getting an elemented unmounted on future `await links[i].getVisibleText()` calls. A [similar fix](#82930) was implemented for visualize and dashboard listing pages. #82930 introduced `listingTable.waitUntilTableIsLoaded` but did not introduce the method in a consistent way. Other methods that search table were not updated to use `listingTable.waitUntilTableIsLoaded`, but instead used `this.header.waitUntilLoadingHasFinished()`. This PR resolved this issue by updating all listingTable methods that search to use `listingTable.waitUntilTableIsLoaded` and then updated `listingTable.waitUntilTableIsLoaded` with a call to `this.header.waitUntilLoadingHasFinished()` #82930 did not update maps_integration tests, only resolving the issue for visualize and dashboard. To avoid future situations where fixes only resolve a few usages, this PR moves selectFilterTags into listing_table and replaces all implementations of selectFilterTags with listingTable.selectFilterTags. While investigating dashboard_integrations test, I found `x-pack/test/functional/apps/dashboard/group2/dashboard_tagging.ts`, which duplicated most of dashboard_integrations test. This PR removes x-pack/test/functional/apps/dashboard/group2/dashboard_tagging.ts adds the unique test case to dashboard_integrations Co-authored-by: kibanamachine <[email protected]>

…t/saved_object_tagging/functional/tests/maps_integration·ts - saved objects tagging (elastic#149356) Fixes elastic#89073 and elastic#106547 Flaky test runner https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1785 Issue with flaky maps_integration.ts is that there was not await when filtering maps list by tags. This resulted in list page refreshing after `const links = await this.find.allByCssSelector('.euiTableRow-isSelectable .euiLink');` and then getting an elemented unmounted on future `await links[i].getVisibleText()` calls. A [similar fix](elastic#82930) was implemented for visualize and dashboard listing pages. elastic#82930 introduced `listingTable.waitUntilTableIsLoaded` but did not introduce the method in a consistent way. Other methods that search table were not updated to use `listingTable.waitUntilTableIsLoaded`, but instead used `this.header.waitUntilLoadingHasFinished()`. This PR resolved this issue by updating all listingTable methods that search to use `listingTable.waitUntilTableIsLoaded` and then updated `listingTable.waitUntilTableIsLoaded` with a call to `this.header.waitUntilLoadingHasFinished()` elastic#82930 did not update maps_integration tests, only resolving the issue for visualize and dashboard. To avoid future situations where fixes only resolve a few usages, this PR moves selectFilterTags into listing_table and replaces all implementations of selectFilterTags with listingTable.selectFilterTags. While investigating dashboard_integrations test, I found `x-pack/test/functional/apps/dashboard/group2/dashboard_tagging.ts`, which duplicated most of dashboard_integrations test. This PR removes x-pack/test/functional/apps/dashboard/group2/dashboard_tagging.ts adds the unique test case to dashboard_integrations Co-authored-by: kibanamachine <[email protected]>

fix flaky test and re-enable it

d216b9e

pgayvallet marked this pull request as ready for review November 9, 2020 11:40

pgayvallet requested a review from a team as a code owner November 9, 2020 11:40

pgayvallet added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 v8.0.0 labels Nov 9, 2020

mshustov reviewed Nov 9, 2020

View reviewed changes

pgayvallet changed the title ~~SO Tagging: fix flaky test and re-enable it~~ SO Tagging: attempt to fix flaky test and re-enable it Nov 9, 2020

pgayvallet added 3 commits November 12, 2020 09:49

wait for table to load before to perform operations

c2d1238

Merge remote-tracking branch 'upstream/master' into kbn-82556-fix-sot…

374814f

…-vis-flacky-ftr-1

move everything out of ciGroup2 for flaky test runner

db86c3b

pgayvallet requested review from a team as code owners November 12, 2020 12:33

pheyos approved these changes Nov 12, 2020

View reviewed changes

pgayvallet added 5 commits November 13, 2020 08:30

add debug block for flaky runner

46da2af

use correct vis name

2909b19

Merge remote-tracking branch 'upstream/master' into kbn-82556-fix-sot…

d37a5b7

…-vis-flacky-ftr-1

remove test sync

c77f036

Revert "move everything out of ciGroup2 for flaky test runner"

5e9a93c

This reverts commit db86c3b

pgayvallet removed request for a team November 16, 2020 13:41

pgayvallet requested a review from mshustov November 16, 2020 13:43

pgayvallet changed the title ~~SO Tagging: attempt to fix flaky test and re-enable it~~ SO Tagging: fix flaky test and re-enable it Nov 16, 2020

pgayvallet added the chore label Nov 17, 2020

mshustov reviewed Nov 17, 2020

View reviewed changes

mshustov approved these changes Nov 17, 2020

View reviewed changes

pgayvallet added 2 commits November 18, 2020 09:08

Merge remote-tracking branch 'upstream/master' into kbn-82556-fix-sot…

5861e50

…-vis-flacky-ftr-1

Merge remote-tracking branch 'upstream/master' into kbn-82556-fix-sot…

61980e9

…-vis-flacky-ftr-1

pgayvallet merged commit b3eefb9 into elastic:master Nov 18, 2020

pgayvallet mentioned this pull request Nov 18, 2020

[7.x] SO Tagging: fix flaky test and re-enable it (#82930) #83653

Merged

nreese mentioned this pull request Jan 24, 2023

[Maps] unskip X-Pack Saved Object Tagging Functional Tests.x-pack/test/saved_object_tagging/functional/tests/maps_integration·ts - saved objects tagging #149356

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SO Tagging: fix flaky test and re-enable it #82930

SO Tagging: fix flaky test and re-enable it #82930

pgayvallet commented Nov 9, 2020

mshustov Nov 9, 2020

pgayvallet Nov 9, 2020 •

edited

Loading

mshustov Nov 9, 2020

pgayvallet Nov 9, 2020

mshustov Nov 10, 2020

pgayvallet Nov 12, 2020 •

edited

Loading

pgayvallet commented Nov 12, 2020

pheyos left a comment •

edited

Loading

pgayvallet commented Nov 16, 2020

mshustov Nov 17, 2020

mshustov Nov 17, 2020

pgayvallet Nov 17, 2020

kibanamachine commented Nov 18, 2020

		await PageObjects.visualize.gotoVisualizationLandingPage();
		await PageObjects.header.waitUntilLoadingHasFinished();

		await PageObjects.visualize.gotoVisualizationLandingPage();
		await listingTable.waitUntilTableIsLoaded();

SO Tagging: fix flaky test and re-enable it #82930

SO Tagging: fix flaky test and re-enable it #82930

Conversation

pgayvallet commented Nov 9, 2020

Summary

mshustov Nov 9, 2020

Choose a reason for hiding this comment

pgayvallet Nov 9, 2020 • edited Loading

Choose a reason for hiding this comment

mshustov Nov 9, 2020

Choose a reason for hiding this comment

pgayvallet Nov 9, 2020

Choose a reason for hiding this comment

mshustov Nov 10, 2020

Choose a reason for hiding this comment

pgayvallet Nov 12, 2020 • edited Loading

Choose a reason for hiding this comment

pgayvallet commented Nov 12, 2020

pheyos left a comment • edited Loading

Choose a reason for hiding this comment

pgayvallet commented Nov 16, 2020

mshustov Nov 17, 2020

Choose a reason for hiding this comment

mshustov Nov 17, 2020

Choose a reason for hiding this comment

pgayvallet Nov 17, 2020

Choose a reason for hiding this comment

kibanamachine commented Nov 18, 2020

💚 Build Succeeded

Metrics [docs]

History

pgayvallet Nov 9, 2020 •

edited

Loading

pgayvallet Nov 12, 2020 •

edited

Loading

pheyos left a comment •

edited

Loading