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 flaky dashboard nested visualizations test #20166

Merged
merged 2 commits into from Jun 27, 2018

Conversation

liza-mae
Copy link
Contributor

Fix for #20158, add wait for page to load in clickEditVisualization method.

@liza-mae liza-mae self-assigned this Jun 22, 2018
@liza-mae liza-mae added test Feature:Dashboard Dashboard related features v6.3.1 labels Jun 22, 2018
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@@ -48,6 +48,7 @@ export function DashboardPageProvider({ getService, getPageObjects }) {
await retry.try(async () => {
await this.showPanelEditControlsDropdownMenu();
await testSubjects.click('dashboardPanelEditLink');
await PageObjects.header.waitUntilLoadingHasFinished();
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if this should be after the check that makes sure the visualization page was loaded, rather than before. I have some suspicions on this function, that it could possibly find the hidden loading indicator on the dashboard page, pass, then check that the visualization page is current, but then the loading indicator would appear again when the visualize page is loading.

Also might be worthwhile to run the ci 4 or 5 times to ensure this stabilizes the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@stacey-gammon I agree in doing more testing I missed doing that for this particular PR, I will run the tests locally in a loop to ensure I can get consistent passing results. In any case, this change does not do any harm, when you click edit visualization link from dashboard, the loading page indicator does show and the method should wait for it. I will check if any other changes are required.

The wait for page to load did not work consistently in a loop.  I also tried a retry in setQuery method which failed also, but was only for one of the tests, the other test did not use that method.  

The only way I could get it to pass in a run 18 times in a loop was to put a sleep in clickEditVisualization.  This change is only for 6.3.
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@liza-mae
Copy link
Contributor Author

The wait for page to load did not work consistently in a loop. I also tried a retry in setQuery method which failed also, but only one of the tests used that, the other test did not.

The only way I could get it to pass in a run 18 times in a loop was to put a sleep in clickEditVisualization. This change is only for 6.3.

Copy link
Contributor

@stacey-gammon stacey-gammon left a comment

Choose a reason for hiding this comment

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

LGTM, good job finding a way to get it to pass!

Chatted with @liza-mae offline and decided that even though a sleep is not a great solution, this is only for 6.3 and the test infrastructure has changed quite a bit in 6.4 and onward (and doesn't seem to have this issue), so no point in spending a lot of time figuring out the discrepancy.

@liza-mae liza-mae merged commit df4ae35 into elastic:6.3 Jun 27, 2018
@liza-mae liza-mae deleted the fix/issue-20158 branch June 27, 2018 18:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Dashboard Dashboard related features test v6.3.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants