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

Fix a few dashboard flaky tests #22901

Closed

Conversation

stacey-gammon
Copy link
Contributor

After discover the "slow 3g" option in Chrome, I was able to reproduce many flaky dashboard issues.

  • clickNewDashboard checked for an add dashboard button, if it didn't exist, it tried until failure to find the button on the prompt. Problem is sometimes it was slow to render the page, so dashboards existed, and the add dashboard button just didn't render yet. But once it failed to find it, it assumed there were no dashboards and only waited to find the "add your first dashboard" prompt button.

  • similarly, gotoDashboardLandingPage first checked for the existence of an item to determine where it was, but if that came back incorrectly (because the page was slow to load), it would fail. Wrapping both checks in a retry should fix this.

  • The second click save button happened too quickly, this waits for the button to be enabled before clicking. Clicking the button when disabled doesn't throw an error. Rather than fix for this specific test, I pushed the check for a disabled button into the find service so any other clicks on an element will wait for the button to be enabled, or will throw an error as unsuccessful.

  • Added in a check to wait for the EuiTableLoading indicator to be removed after searching for a dashboard on the listing page. If it's in the process of loading, some clicks can miss their elements.

Fixes #22409 ... probably many more! 🎉

@@ -24,79 +24,76 @@ export default function ({ getService, getPageObjects }) {
const retry = getService('retry');
const PageObjects = getPageObjects(['common', 'discover', 'visualize', 'header']);

describe('visualize app', function describeIndexTests() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

just removed the unnecessary wrapper

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@stacey-gammon
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@stacey-gammon
Copy link
Contributor Author

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon
Copy link
Contributor Author

Failed on a flaky test

UI Functional Tests.test/functional/apps/dashboard/_dashboard_state·js.dashboard app using legacy data dashboard state Overriding colors on an area chart is preserved

jenkins, test this

@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon
Copy link
Contributor Author

Overriding colors on an area chart is preserved failure again. This one I guess needs to be addressed now, something in this PR may be causing it to be hit more frequently.

Generalize the fix and include some other flakiness encountered with slow network connection

dont wrap in a retry or stale element checks will get caught

dont click disabled go button

Skip test hitting legit input control bug
@elasticmachine
Copy link
Contributor

💔 Build Failed

@stacey-gammon
Copy link
Contributor Author

Failed on UI Functional Tests.test/functional/apps/visualize/_data_table·js.visualize app data table should show correct data for a data table with top hits

jenkins, test this

@LeeDr
Copy link
Contributor

LeeDr commented Sep 13, 2018

This last failure was;
"FAILURE test/functional/apps/visualize/_data_table·js visualize app data table should show correct data for a data table with top hits"

That test has only failed 2 other times in the last 30 days. Both of those were on Sept 11th on @ppisljar's PR #22756 (which is now merged) but I don't know if his PR had anything to do with that test. I don't see any changes to tests in that PR.

If this PR is adding the check to see if something is disabled then this failure might be related (this is from this PR's last failing build);

           │ debg TestSubjects.find(globalLoadingIndicator-hidden)
           │ debg findByCssSelector [data-test-subj~="globalLoadingIndicator-hidden"]
           │ debg clickByCssSelector(button[data-test-subj="toggleEditor"])
           │ debg findByCssSelector button[data-test-subj="toggleEditor"]
           │ debg clickWhenNotDisabled
           │ debg clickByCssSelector(
           │              [group-name="metrics"]
           │              vis-editor-agg-params:not(.ng-hide)
           │              
           │              .agg-select
           │            )
           │ debg findByCssSelector 
           │              [group-name="metrics"]
           │              vis-editor-agg-params:not(.ng-hide)
           │              
           │              .agg-select
           │            
           │ debg clickWhenNotDisabled
           │ debg findByCssSelector 
           │              [group-name="metrics"]
           │              vis-editor-agg-params:not(.ng-hide)
           │              
           │              .agg-select
           │             input.ui-select-search
           │ debg ... sleep(500) start
           │ debg ... sleep(500) end
           │ debg clickByCssSelector(
           │              [group-name="metrics"]
           │              vis-editor-agg-params:not(.ng-hide)
           │              
           │              .field-select
           │            )
           │ debg findByCssSelector 
           │              [group-name="metrics"]
           │              vis-editor-agg-params:not(.ng-hide)
           │              
           │              .field-select
           │            
           │ debg clickWhenNotDisabled
           │ debg findByCssSelector 
           │              [group-name="metrics"]
           │              vis-editor-agg-params:not(.ng-hide)
           │              
           │              .field-select
           │             input.ui-select-search
           │ debg ... sleep(500) start
           │ debg ... sleep(500) end
           │ debg TestSubjects.click(visualizeEditorRenderButton)
           │ debg clickByCssSelector([data-test-subj~="visualizeEditorRenderButton"])
           │ debg findByCssSelector [data-test-subj~="visualizeEditorRenderButton"]
           │ debg clickWhenNotDisabled
           │ debg Element is disabled, try again
           │ debg Element is disabled, try again
           │ debg Element is disabled, try again
           │ debg Element is disabled, try again

It doesn't look like we captured a screenshot or page source for some reason on this failure so it's hard to know why the visualizeEditorRenderButton was not enabled.

@stacey-gammon
Copy link
Contributor Author

it might be a legit bug. I ran into a situation last week where the play button became disabled and it wasn't becoming enabled. I couldn't repro through, once I refreshed the page.

I won't submit until these issues are worked out, but this is partly why I'm concerned that swapping out an entire test backend for all tests is going to be a huge issue. Slight timing changes can expose legit bugs and more flakiness.

Look at this PR: #22787

When I ran the _dashboard_filtering test suite multiple times in a row, a test farther down the line failed for an unrelated error. I would like to dig into that more to get to the root of the problem, but it seems very easy to break a bunch of tests by just changing the timing of things up a bit. That PR also hit quite a few unrelated flaky tests, though at least stopped failing consistently when I went back to only a single test run.

Overriding colors on an area chart is preserved also seems to be getting hit more frequently in this PR. I might explore breaking up some of the improvements in this PR to get some of them checked in.

@elasticmachine
Copy link
Contributor

💔 Build Failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dashboard app using legacy data dashboard save Saves on confirm duplicate title warning
3 participants