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

[QA][fix failing test] Refactor to ui settings for timepicker #114182

Closed
wants to merge 2 commits into from

Conversation

wayneseymour
Copy link
Member

@wayneseymour wayneseymour commented Oct 6, 2021

[skip-ci]

Use kibanaServer.uiSettings.replace()
instead of PageObjects.timePicker.setDefaultAbsoluteRange()

Resolves #60559

Helps with: #113998

@wayneseymour wayneseymour added release_note:skip Skip the PR/issue when compiling release notes v7.16.0 v8.0.0 auto-backport Deprecated - use backport:version if exact versions are needed buildkite-ci labels Oct 6, 2021
Use ui settings instead of setting absolute range
for timepicker.
@wayneseymour wayneseymour force-pushed the refactor/timepicker-use branch from b8a8c08 to a20a71f Compare October 7, 2021 13:45
Use ui settings instead of setting absolute range
for timepicker.
@wayneseymour wayneseymour force-pushed the refactor/timepicker-use branch from 74fd8bb to fd65ef0 Compare October 7, 2021 14:07
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

Metrics [docs]

✅ unchanged

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

@wayneseymour wayneseymour marked this pull request as ready for review October 7, 2021 15:59
@wayneseymour wayneseymour requested a review from LeeDr October 7, 2021 15:59
@dmlemeshko
Copy link
Member

I like the general idea of replacing UI interaction via Webdriver with kbnServer, but I think doing it for flaky (potentially buggy) tests is not the right approach.
Particular test looks like a potential UI bug that just hard to reproduce and if we make a switch, we will never find it out.

I think we should migrate stable tests to kbnServer.uiSettings.replace usage, for flaky ones I would talk to developers and do proper investigation.

@wayneseymour
Copy link
Member Author

I like the general idea of replacing UI interaction via Webdriver with kbnServer, but I think doing it for flaky (potentially buggy) tests is not the right approach. Particular test looks like a potential UI bug that just hard to reproduce and if we make a switch, we will never find it out.

I think we should migrate stable tests to kbnServer.uiSettings.replace usage, for flaky ones I would talk to developers and do proper investigation.

Add, that makes a lot of sense man. I'll follow up

@wayneseymour
Copy link
Member Author

Sadly there is no codeowner for this file, in .github/CODEOWNERS.
I'll check kibana-stats

@wayneseymour
Copy link
Member Author

wayneseymour commented Oct 7, 2021

@kobelb Hello there, my team and I are in an effort to fixup tests that are flaky due to the timepicker.

Dima brought up a good point above, and I'd like your input.

@kobelb
Copy link
Contributor

kobelb commented Oct 7, 2021

+1 to what @dmlemeshko's recommendation.

In general, I think that we should be doing test setup/teardown as much as possible using kbnServer.uiSettings.replace because it's faster and can help us when writing tests in such a way that they only fail for one reason. However, there's a risk in switching to using kbnServer.uiSettings.replace when there's flakiness as there might be a difficult to reproduce bug that we're overlooking.

@wayneseymour
Copy link
Member Author

+1 to what @dmlemeshko's recommendation.

In general, I think that we should be doing test setup/teardown as much as possible using kbnServer.uiSettings.replace because it's faster and can help us when writing tests in such a way that they only fail for one reason. However, there's a risk in switching to using kbnServer.uiSettings.replace when there's flakiness as there might be a difficult to reproduce bug that we're overlooking.

Ok, makes sense.
I presumed the test was the problem.
Do you think it's the app instead?
If so, whom might I contact to resolve this?

@kobelb
Copy link
Contributor

kobelb commented Oct 7, 2021

Based on the CI logs from when this test failed (the last couple lines repeat for a while until it times out):

[00:20:06]                 │ debg Validating 'startDate' - expected: 'Sep 19, 2015 @ 06:31:44.000, actual: Sep 19, 2015 @ 06:31:44.000'
[00:20:06]                 │ debg Waiting up to 20000ms for Timepicker popover to close...
[00:20:06]                 │ debg TestSubjects.exists(superDatePickerAbsoluteDateInput)
[00:20:06]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="superDatePickerAbsoluteDateInput"]') with timeout=2500
[00:20:06]                 │ debg TestSubjects.exists(superDatePickerAbsoluteDateInput)
[00:20:06]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="superDatePickerAbsoluteDateInput"]') with timeout=2500
[00:20:07]                 │ debg TestSubjects.exists(superDatePickerAbsoluteDateInput)
[00:20:07]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="superDatePickerAbsoluteDateInput"]') with timeout=2500
[00:20:07]                 │ debg TestSubjects.exists(superDatePickerAbsoluteDateInput)
[00:20:07]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="superDatePickerAbsoluteDateInput"]') with timeout=2500

It looks like the timepicker was stuck in an open state. So, it's possible that this is a bug with the timepicker itself or the Discover application. I'd start with the @elastic/kibana-data-discovery team to rule out it being a bug with the Discover application before looking at the timepicker as the culprit.

@LeeDr LeeDr added the v8.1.0 label Nov 3, 2021
@LeeDr LeeDr added v7.16.1 and removed v7.16.0 labels Nov 16, 2021
@mistic mistic added the v7.17.0 label Dec 16, 2021
@sophiec20 sophiec20 deleted the refactor/timepicker-use branch July 29, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed buildkite-ci release_note:skip Skip the PR/issue when compiling release notes v7.16.1 v7.17.0 v8.0.0 v8.1.0
Projects
None yet
6 participants