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.

Sign up for GitHub

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

quick check of current URL to skip timepicker #146462

Merged
merged 20 commits into from
Dec 4, 2022
Merged

quick check of current URL to skip timepicker #146462

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
3702547
quick check of current URL to skip timepicker
Nov 28, 2022
3a05844
[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…
kibanamachine Nov 28, 2022
84a0bf6
changed setAbsoluteTime to a wrapper that checks URL
Nov 29, 2022
1b69342
resolve merge
Nov 29, 2022
989a21d
Merge branch 'main' of github.com:elastic/kibana into timepickerCheck…
Nov 29, 2022
f90053c
Merge branch 'main' into timepickerChecksUrl
Nov 29, 2022
17ca23e
Merge branch 'main' into timepickerChecksUrl
Nov 29, 2022
d3a2a7c
Merge branch 'main' into timepickerChecksUrl
Nov 30, 2022
bb010e5
add force=false default param to setAbsoluteTime
Nov 30, 2022
d7dbdde
Merge branch 'timepickerChecksUrl' of github.com:LeeDr/kibana into ti…
Nov 30, 2022
15a1d92
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Nov 30, 2022
d535634
fix conditional branch, decoodeURL
Nov 30, 2022
7465da6
fix conditional branch, decoodeURL
Nov 30, 2022
37c2990
[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…
kibanamachine Nov 30, 2022
cb1b50f
Merge branch 'main' into timepickerChecksUrl
Nov 30, 2022
c3375fc
Merge branch 'main' into timepickerChecksUrl
Dec 1, 2022
bee5c67
Merge branch 'main' into timepickerChecksUrl
dmlemeshko Dec 2, 2022
4f4e096
Merge branch 'main' into timepickerChecksUrl
dmlemeshko Dec 2, 2022
a681d41
revert changes in area chart tests
dmlemeshko Dec 4, 2022
61056f4
Merge branch 'main' into timepickerChecksUrl
dmlemeshko Dec 4, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions test/functional/apps/visualize/replaced_vislib_chart_types/_area_chart.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,7 +487,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
beforeEach(async () => {
const fromTime = 'Sep 20, 2015 @ 00:00:00.000';
const toTime = 'Sep 20, 2015 @ 23:30:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime, true);
});

it('should update collapsed accordion label when time range is changed', async () => {
Expand All @@ -498,7 +498,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(accordionLabelText).to.include.string('per 30 minutes');
const fromTime = 'Sep 20, 2015 @ 08:30:00.000';
const toTime = 'Sep 20, 2015 @ 23:30:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime, true);
accordionLabelText = await accordionLabel.getVisibleText();
expect(accordionLabelText).to.include.string('per 10 minutes');
});
Expand Down Expand Up @@ -530,7 +530,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(helperScaledLabelText).to.include.string('to 10 minutes');
const fromTime = 'Sep 20, 2015 @ 22:30:00.000';
const toTime = 'Sep 20, 2015 @ 23:30:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime, true);
helperScaledLabelText = await testSubjects.getVisibleText('currentlyScaledText');
expect(helperScaledLabelText).to.include.string('to 30 seconds');
});
Expand All @@ -546,7 +546,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(helperScaledLabelText).to.include.string('to 10 minutes');
const fromTime = 'Sep 20, 2015 @ 21:30:00.000';
const toTime = 'Sep 20, 2015 @ 23:30:00.000';
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime, true);
helperScaledLabelText = await testSubjects.getVisibleText('currentlyScaledText');
expect(helperScaledLabelText).to.include.string('to minute');
});
Expand Down
15 changes: 14 additions & 1 deletion test/functional/page_objects/time_picker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,21 @@ export class TimePickerPageObject extends FtrService {
/**
* @param {String} fromTime MMM D, YYYY @ HH:mm:ss.SSS
* @param {String} toTime MMM D, YYYY @ HH:mm:ss.SSS
* @param {Boolean} force time picker force update, default is false
*/
public async setAbsoluteRange(fromTime: string, toTime: string) {
public async setAbsoluteRange(fromTime: string, toTime: string, force = false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Under what circumstances does it make sense to force?

Copy link
Member

Choose a reason for hiding this comment

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

Off the top of my head, anytime flakiness is encountered (timepicker can be notorious sometimes).
This pr is a pr from a former elastic employee (our boss) and we did discuss at some length.
Further, there is some use of force flag within this pr.

@dmlemeshko may have more to weigh in on this.

Copy link
Member

Choose a reason for hiding this comment

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

@andrewctate , it is a good question. For some reason Lee picked area chart, maybe he was doing some testing.
You might know more about this test, and if there is no good reason re-resetting the time picker with already the same range, I can push an update and remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dmlemeshko I agree with Andrew, this test was not flaky. I might miss something here but I would propose to remove it!

if (!force) {
const currentUrl = decodeURI(await this.browser.getCurrentUrl());
const DEFAULT_DATE_FORMAT = 'MMM D, YYYY @ HH:mm:ss.SSS';
const startMoment = moment.utc(fromTime, DEFAULT_DATE_FORMAT).toISOString();
const endMoment = moment.utc(toTime, DEFAULT_DATE_FORMAT).toISOString();
if (currentUrl.includes(`time:(from:'${startMoment}',to:'${endMoment}'`)) {
this.log.debug(
`We already have the desired start (${fromTime}) and end (${toTime}) in the URL, returning from setAbsoluteRange`
);
return;
}
}
this.log.debug(`Setting absolute range to ${fromTime} to ${toTime}`);
await this.showStartEndTimes();
let panel!: WebElementWrapper;
Expand Down