-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[test/page_objects] validate selected dates for timepicker #113597
[test/page_objects] validate selected dates for timepicker #113597
Conversation
Running 100x x-pack ciGroup1 to check if PR fixes flakiness https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1973 |
Flaky test runner passed 100x, retest PR |
@elasticmachine merge upstream |
@@ -116,23 +116,38 @@ export class TimePickerPageObject extends FtrService { | |||
public async setAbsoluteRange(fromTime: string, toTime: string) { | |||
this.log.debug(`Setting absolute range to ${fromTime} to ${toTime}`); | |||
await this.showStartEndTimes(); | |||
let panel!: WebElementWrapper; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nit:
Do we need the non-null assertion operator here?
I don't think WebElementWrapper can be null.
Did your compiler complain?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type checker on CI is not happy without it :)
'superDatePickerendDatePopoverButton' | ||
); | ||
this.log.debug(`Validating 'endDate' is set to '${toTime}'`); | ||
return toTime === actualToTime; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smart!
9863ef2
to
02d6a85
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
vis editors code LGTM, code review only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've linked a couple of more currently skipped tests in #113520. Could we maybe try unskipping them too and run them through the Flaky FTR to validate them? Other than that, Data Discovery Code LGTM
Actually this one #113496 is already unskipped on master. We just need to validate that this fix also stabilizes it in order to not skip it again :) |
Agree on checking other skipped tests mentioned in #113520 on flaky-test-runner. |
cc438c6
to
a16ccf9
Compare
💛 Build succeeded, but was flaky
Test FailuresKibana Pipeline / general / Chrome UI Functional Tests.test/functional/apps/discover/_indexpattern_without_timefield·ts.discover app indexpattern without timefield should switch between with and without timefield using the browser back buttonStandard Out
Stack Trace
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
…13597) * [test/page_objects] validate selected dates for timepicker * unskip flaky test * fix types check failure * update tests * update message * unskip more tests
…13597) * [test/page_objects] validate selected dates for timepicker * unskip flaky test * fix types check failure * update tests * update message * unskip more tests
Summary
Adding validation+retry logic for time picker absolute value selection.
Partial solution for #113520
Closes #113067
Closes #104578
Closes #60559
Checklist
Delete any items that are not applicable to this PR.
Risk Matrix
Delete this section if it is not applicable to this PR.
Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.
When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:
For maintainers