Skip to content

Add 1/2 second sleep to fix test#37903

Merged
LeeDr merged 2 commits into
elastic:masterfrom
LeeDr:sleepDashboardCancel_22299
Jun 4, 2019
Merged

Add 1/2 second sleep to fix test#37903
LeeDr merged 2 commits into
elastic:masterfrom
LeeDr:sleepDashboardCancel_22299

Conversation

@LeeDr
Copy link
Copy Markdown

@LeeDr LeeDr commented Jun 3, 2019

Summary

Fixes: #22299

The test is "dashboard app using current data dashboard view edit mode shows lose changes warning and loses changes on confirmation when a new vis is added"
in test\functional\apps\dashboard\view_edit.js

This PR adds a 1/2 second sleep in between clicking cancel from edit mode on a dashboard and the click to confirm that cancel. Although this test usually passes on Jenkins, I've never had it pass for me locally using either Chrome or Firefox browser. With the small sleep it always passes for me. Without the sleep, it looks like the cancel doesn't happen. The 3rd panel added to the dashboard is still there and so the test fails with Error: expected 3 to sort of equal 2

I think it's actually a product bug. My thought is that the confirm cancel modal dialog appears and the test is clicking confirm before some event handler is attached to the button? But since real users won't ever click it that fast, I think it's OK to just fix this test.

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

- [ ] This was checked for cross-browser compatibility, including a check against IE11
- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
- [ ] Documentation was added for features that require explanation or tutorials

For maintainers

- [x] This was checked for breaking API changes and was labeled appropriately
- [] This includes a feature addition or change that requires a release note and was labeled appropriately

Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

@LeeDr What if you try await renderable.waitForRender(); in place of sleep?
I wonder if data-render-complete attribute is changing at the moment and we can use it.

Otherwise, I'm good with sleep :)

@elasticmachine
Copy link
Copy Markdown
Contributor

💔 Build Failed

@LeeDr
Copy link
Copy Markdown
Author

LeeDr commented Jun 3, 2019

@dmlemeshko see comments back on this issue #22299

I don't know if we wait for the modal dialog to render? I don't know if there's something in the DOM we can check to know when it's ready.

Copy link
Copy Markdown
Contributor

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

I checked it locally and agree with the proposed solution

@elasticmachine
Copy link
Copy Markdown
Contributor

💚 Build Succeeded

@LeeDr
Copy link
Copy Markdown
Author

LeeDr commented Jun 4, 2019

@stacey-gammon I'm not sure where you are on investigating this issue, but I'm going to go ahead and merge this one-line temporary work-around to get the test passing. When we figure out why the cancel changes isn't working we can remove this sleep.

@LeeDr LeeDr merged commit e9938a5 into elastic:master Jun 4, 2019
jgowdyelastic pushed a commit that referenced this pull request Jun 4, 2019
@stacey-gammon
Copy link
Copy Markdown

Thanks @LeeDr, I have no time to investigate, so sg to stabilize tests.

@LeeDr LeeDr deleted the sleepDashboardCancel_22299 branch August 20, 2020 17:56
patrykkopycinski pushed a commit to patrykkopycinski/kibana that referenced this pull request May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dashboard app using current data dashboard view edit mode shows lose changes warning and loses changes on confirmation when a new vis is added

4 participants