quick check of current URL to skip timepicker #146462

LeeDr · 2022-11-28T19:15:23Z

Summary

We have some tests which set the default time but then also call setAbsoluteTime in every test. This PR adds code in setAbsoluteTime to quickly check if the desired start and end (from and to) times are already set, and if so, just return. The end result should be less time spent on functional tests. It could also reduce occasional flakiness setting the time.

…-ref HEAD~1..HEAD --fix'

LeeDr · 2022-11-28T21:38:55Z

The visualize app failure "FTR Configs #50 / visualize app - new charts library visualize area charts date histogram interval expanded accordion should update scaled label text after custom interval is set and time range is changed" is a case where the timepicker already had the time that was being set so it didn't set it.
See the log "We already have the desired start (Sep 20, 2015 @ 00:00:00.000) and end (Sep 20, 2015 @ 23:30:00.000) in the URL, returning from setAbsoluteRange"

So it might be appropriate to change the test to pick a slightly different time (even just 1 second different) so that the custom interval reset as the test expects. I'll try that.

[00:04:48]           └-> should update scaled label text after custom interval is set and time range is changed
[00:04:48]             └-> "before each" hook: global before each for "should update scaled label text after custom interval is set and time range is changed"
[00:04:48]             └-> "before each" hook for "should update scaled label text after custom interval is set and time range is changed"
[00:04:48]               │ debg We already have the desired start (Sep 20, 2015 @ 00:00:00.000) and end (Sep 20, 2015 @ 23:30:00.000) in the URL, returning from setAbsoluteRange
[00:04:48]             │ debg visEditor.setInterval(10s, {custom, 2, false})
[00:04:48]             │ debg comboBox.setCustom, comboBoxSelector: visEditorInterval, value: 10s
[00:04:48]             │ debg TestSubjects.find(visEditorInterval)
[00:04:48]             │ debg Find.findByCssSelector('[data-test-subj="visEditorInterval"]') with timeout=10000
[00:04:49]             │ debg TestSubjects.exists(~comboBoxOptionsList)
[00:04:49]             │ debg Find.existsByDisplayedByCssSelector('[data-test-subj~="comboBoxOptionsList"]') with timeout=50
[00:04:49]             │ debg --- retry.tryForTime error: [data-test-subj~="comboBoxOptionsList"] is not displayed
[00:04:49]             │ debg TestSubjects.clickWhenNotDisabledWithoutRetry(visualizeEditorRenderButton)
[00:04:49]             │ debg Find.clickByCssSelectorWhenNotDisabledWithoutRetry([data-test-subj="visualizeEditorRenderButton"], timeout=10000)
[00:04:49]             │ debg Find.findByCssSelector('[data-test-subj="visualizeEditorRenderButton"]') with timeout=10000
[00:04:50]             │ debg Find.existsByCssSelector('[data-test-subj="currentlyScaledText"]') with timeout=2500
[00:04:50]             │ debg TestSubjects.getVisibleText(currentlyScaledText)
[00:04:50]             │ debg TestSubjects.find(currentlyScaledText)
[00:04:50]             │ debg Find.findByCssSelector('[data-test-subj="currentlyScaledText"]') with timeout=10000
[00:04:50]             │ info Taking screenshot "/var/lib/buildkite-agent/builds/kb-n2-4-spot-3689532d47ccab6a/elastic/kibana-pull-request/kibana/test/functional/screenshots/failure/visualize app - new charts library visualize area charts date histogram interval-173123d4f783ab6f4a5ca31b620c54cd9c12a04e697316f7120bf61bd9a25432.png"
[00:04:50]             │ info Current URL is: http://localhost:5620/app/visualize#/edit/Visualization-AreaChart?_g=(filters:!(),refreshInterval:(pause:!t,value:0),time:(from:%272015-09-20T22:30:00.000Z%27,to:%272015-09-20T23:30:00.000Z%27))&_a=(filters:!(),linked:!f,query:(language:lucene,query:%27*%27),uiState:(),vis:(aggs:!((enabled:!t,id:%271%27,params:(emptyAsNull:!f),schema:metric,type:count),(enabled:!t,id:%272%27,params:(drop_partials:!f,extendToTimeRange:!f,extended_bounds:(),field:%27@timestamp%27,interval:%2710s%27,min_doc_count:1,scaleMetricValues:!f,timeRange:(from:%272015-09-20T22:30:00.000Z%27,to:%272015-09-20T23:30:00.000Z%27),useNormalizedEsInterval:!t,used_interval:%2730s%27),schema:segment,type:date_histogram)),params:(addLegend:!t,addTimeMarker:!f,addTooltip:!t,categoryAxes:!((id:CategoryAxis-1,labels:(filter:!t,show:!t,truncate:100),position:bottom,scale:(type:linear),show:!t,style:(),title:(),type:category)),detailedTooltip:!t,fittingFunction:zero,grid:(categoryLines:!f),isVislibVis:!t,labels:(),legendPosition:right,legendSize:auto,maxLegendLines:1,palette:(name:kibana_palette,type:palette),radiusRatio:9,seriesParams:!((data:(id:%271%27,label:Count),drawLinesBetweenPoints:!t,interpolate:linear,lineWidth:2,mode:stacked,show:!t,showCircles:!t,type:area,valueAxis:ValueAxis-1)),thresholdLine:(color:%23E7664C,show:!f,style:full,value:10,width:1),times:!(),truncateLegend:!t,type:area,valueAxes:!((id:ValueAxis-1,labels:(filter:!f,rotate:0,show:!t,truncate:100),name:LeftAxis-1,position:left,scale:(mode:normal,type:linear),show:!t,style:(),title:(text:Count),type:value))),title:%27Visualization%20AreaChart%27,type:area))
[00:04:50]             │ info Saving page source to: /var/lib/buildkite-agent/builds/kb-n2-4-spot-3689532d47ccab6a/elastic/kibana-pull-request/kibana/test/functional/failure_debug/html/visualize app - new charts library visualize area charts date histogram interval-173123d4f783ab6f4a5ca31b620c54cd9c12a04e697316f7120bf61bd9a25432.html
[00:04:50]             └- ✖ fail: visualize app - new charts library visualize area charts date histogram interval expanded accordion should update scaled label text after custom interval is set and time range is changed
[00:04:50]             │      Error: expected 'Currently scaled to 30 seconds' to contain 'to 10 minutes'
[00:04:50]             │       at Assertion.assert (node_modules/@kbn/expect/expect.js:100:11)
[00:04:50]             │       at Assertion.string [as contain] (node_modules/@kbn/expect/expect.js:442:10)
[00:04:50]             │       at Context.<anonymous> (test/functional/apps/visualize/replaced_vislib_chart_types/_area_chart.ts:546:52)
[00:04:50]             │       at runMicrotasks (<anonymous>)
[00:04:50]             │       at processTicksAndRejections (node:internal/process/task_queues:96:5)
[00:04:50]             │       at Object.apply (node_modules/@kbn/test/target_node/src/functional_test_runner/lib/mocha/wrap_function.js:78:16)
[00:04:50]             │

UPDATE: I resolved this by changing setAbsoluteTime to be a wrapper around the old functionality and renaming setAbsoluteTime to setAbsoluteTimeAlways. But I'm thinking a better name might be setAbsoluteTimeForce?

…sUrl

LeeDr · 2022-11-30T00:13:23Z

Here's a bit of data from main branch where I added timestamps to the test output with a set of Lens tests running which keep setting the timepicker to the same time. They're all setting the same time in each test. Each call to setAbsoluteTime takes about 3 seconds. One lens test indirectly calls setAbsoluteTime 23 times! And all combined lens tests call it 82 times.

[17:46:23.799016000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:46:27.102308000] │ debg ------ finished setAbsoluteTime --------

[17:47:27.098836000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:47:30.560366000] │ debg ------ finished setAbsoluteTime --------

[17:48:36.036481000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:48:38.981191000] │ debg ------ finished setAbsoluteTime --------

[17:49:23.556883000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:49:26.741647000] │ debg ------ finished setAbsoluteTime --------

[17:50:26.039886000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:50:28.951072000] │ debg ------ finished setAbsoluteTime --------

[17:51:11.732546000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:51:15.133475000] │ debg ------ finished setAbsoluteTime --------

[17:51:44.732817000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:51:47.905630000] │ debg ------ finished setAbsoluteTime --------

[17:52:45.347602000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:52:48.757343000] │ debg ------ finished setAbsoluteTime --------

[17:53:48.883881000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:53:51.750318000] │ debg ------ finished setAbsoluteTime --------

[17:54:34.861078000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:54:38.045885000] │ debg ------ finished setAbsoluteTime --------

[17:55:45.996329000] │ debg Setting absolute range to Sep 19, 2015 @ 06:31:44.000 to Sep 23, 2015 @ 18:31:44.000
[17:55:48.866432000] │ debg ------ finished setAbsoluteTime --------

Compared to this PR (I added debug logging just to see the start and end of the URL check) which shows that it only takes less than 1s to check if the desired dates are in the URL;

[18:09:29.618310000] │ debg ------------- start here ------------------
[18:09:29.623921000] │ debg We already have the desired start (Sep 19, 2015 @ 06:31:44.000) and end (Sep 23, 2015 @ 18:31:44.000) in the URL, returning from setAbsoluteRange
[18:09:29.628524000] │ debg ------------- end here ------------------

[18:10:24.026283000] │ debg ------------- start here ------------------
[18:10:24.373660000] │ debg We already have the desired start (Sep 19, 2015 @ 06:31:44.000) and end (Sep 23, 2015 @ 18:31:44.000) in the URL, returning from setAbsoluteRange
[18:10:24.380117000] │ debg ------------- end here ------------------

[18:10:35.874394000] │ debg ------------- start here ------------------
[18:10:36.020148000] │ debg We already have the desired start (Sep 19, 2015 @ 06:31:44.000) and end (Sep 23, 2015 @ 18:31:44.000) in the URL, returning from setAbsoluteRange
[18:10:36.025582000] │ debg ------------- end here ------------------

[18:10:53.187577000] │ debg ------------- start here ------------------
[18:10:53.197406000] │ debg We already have the desired start (Sep 19, 2015 @ 06:31:44.000) and end (Sep 23, 2015 @ 18:31:44.000) in the URL, returning from setAbsoluteRange
[18:10:53.203220000] │ debg ------------- end here ------------------

LeeDr · 2022-11-30T00:27:13Z

test/functional/page_objects/time_picker.ts

+   * @param {String} fromTime MMM D, YYYY @ HH:mm:ss.SSS
+   * @param {String} toTime MMM D, YYYY @ HH:mm:ss.SSS
+   */
+  public async setAbsoluteRangeAlways(fromTime: string, toTime: string) {


I'm not in love with the name setAbsoluteTimeAlways and thought setAbsoluteTimeForce might be better?

Possible alternative

/** * @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, force = false) {

Maybe It won't be bad to call function few time like
await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime, true);

dmlemeshko · 2022-11-30T11:18:24Z

test/functional/page_objects/time_picker.ts

+    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(startMoment) && currentUrl.includes(endMoment)) {


Maybe I'm late to jump here, but just one thought. Maybe we can make a more strict times check?

currentUrl.includes(`time:(from:'${startMoment}',to:'${endMoment}')`)

I guess the format is consistent across the plugins, right?

Before I even read the rest of this, I wonder if it's worth to strongly type the format.
Actually, I'm hoping that's been done somewhere in this repo already.

I got @dmlemeshko 's suggestion to work. I had to decodeURI.

…mepickerChecksUrl

…-fix'

elasticmachine · 2022-11-30T21:27:11Z

Pinging @elastic/kibana-qa (Team:QA)

dmlemeshko · 2022-12-02T12:49:19Z

buildkite test this

dmlemeshko · 2022-12-02T15:06:34Z

I took some time testing it locally and seems like it works perfect:

        await PageObjects.common.navigateToApp('lens');
        await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
        await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
        await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);
        await PageObjects.timePicker.setAbsoluteRange(fromTime, toTime);

outputs

         │ debg We already have the desired start (Jun 17, 2022 @ 00:00:00.000) and end (Jun 23, 2022 @ 00:00:00.000) in the URL, returning from setAbsoluteRange
         │ debg We already have the desired start (Jun 17, 2022 @ 00:00:00.000) and end (Jun 23, 2022 @ 00:00:00.000) in the URL, returning from setAbsoluteRange
         │ debg We already have the desired start (Jun 17, 2022 @ 00:00:00.000) and end (Jun 23, 2022 @ 00:00:00.000) in the URL, returning from setAbsoluteRange

so time picker is set only first time, the actual url was http://localhost:5620/app/lens#/?_g=(filters:!(),refreshInterval:(pause:!t,value:60000),time:(from:%272022-06-17T00:00:00.000Z%27,to:%272022-06-23T00:00:00.000Z%27

drewdaemon · 2022-12-02T15:58:16Z

test/functional/page_objects/time_picker.ts

   */
-  public async setAbsoluteRange(fromTime: string, toTime: string) {
+  public async setAbsoluteRange(fromTime: string, toTime: string, force = false) {


Under what circumstances does it make sense to force?

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.

@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.

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

dmlemeshko · 2022-12-04T13:53:54Z

buildkite test this

kibana-ci · 2022-12-04T14:40:42Z

💚 Build Succeeded

Buildkite Build
Commit: 61056f4

Metrics [docs]

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	59	65	+6
`osquery`	109	115	+6
`securitySolution`	443	449	+6
total			+20

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	68	74	+6
`osquery`	110	117	+7
`securitySolution`	520	526	+6
total			+21

History

💚 Build #92292 succeeded 4f4e096
💚 Build #91956 succeeded cb1b50f
💔 Build #91934 failed 37c2990

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

kibanamachine · 2022-12-04T20:51:14Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

## Summary We have some tests which set the default time but then also call setAbsoluteTime in every test. This PR adds code in setAbsoluteTime to quickly check if the desired start and end (from and to) times are already set, and if so, just return. The end result should be less time spent on functional tests. It could also reduce occasional flakiness setting the time. Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Dzmitry Lemechko <dzmitry.lemechko@elastic.co> (cherry picked from commit 1734b0e)

kibanamachine · 2022-12-04T20:55:16Z

💚 All backports created successfully

Status	Branch	Result
✅	8.6

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.6`: - [quick check of current URL to skip timepicker (#146462)](#146462)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Lee Drengenberg <lee.drengenberg@elastic.co>

Lee Drengenberg and others added 2 commits November 28, 2022 13:13

quick check of current URL to skip timepicker

3702547

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

3a05844

…-ref HEAD~1..HEAD --fix'

Lee Drengenberg added 3 commits November 29, 2022 11:27

changed setAbsoluteTime to a wrapper that checks URL

84a0bf6

resolve merge

1b69342

Merge branch 'main' of github.com:elastic/kibana into timepickerCheck…

989a21d

…sUrl

LeeDr added Team:QA Team label for QA Team test_ui_functional test_xpack_functional release_note:skip Skip the PR/issue when compiling release notes backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 29, 2022

Lee Drengenberg added 2 commits November 29, 2022 13:27

Merge branch 'main' into timepickerChecksUrl

f90053c

Merge branch 'main' into timepickerChecksUrl

17ca23e

Merge branch 'main' into timepickerChecksUrl

d3a2a7c

LeeDr commented Nov 30, 2022

View reviewed changes

dmlemeshko reviewed Nov 30, 2022

View reviewed changes

Lee Drengenberg and others added 6 commits November 30, 2022 09:58

add force=false default param to setAbsoluteTime

bb010e5

Merge branch 'timepickerChecksUrl' of github.com:LeeDr/kibana into ti…

d7dbdde

…mepickerChecksUrl

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

15a1d92

…-fix'

fix conditional branch, decoodeURL

d535634

fix conditional branch, decoodeURL

7465da6

[CI] Auto-commit changed files from 'node scripts/eslint --no-cache -…

37c2990

…-fix'

elastic deleted a comment from kibana-ci Nov 30, 2022

Merge branch 'main' into timepickerChecksUrl

cb1b50f

LeeDr marked this pull request as ready for review November 30, 2022 21:27

LeeDr requested a review from a team as a code owner November 30, 2022 21:27

LeeDr mentioned this pull request Nov 30, 2022

remove calls to set the timepicker in Lens tests, use default time #145656

Closed

Merge branch 'main' into timepickerChecksUrl

c3375fc

dmlemeshko added 2 commits December 2, 2022 10:03

Merge branch 'main' into timepickerChecksUrl

bee5c67

Merge branch 'main' into timepickerChecksUrl

4f4e096

dmlemeshko requested a review from wayneseymour December 2, 2022 12:52

wayneseymour approved these changes Dec 2, 2022

View reviewed changes

drewdaemon reviewed Dec 2, 2022

View reviewed changes

dmlemeshko added 2 commits December 4, 2022 14:13

revert changes in area chart tests

a681d41

Merge branch 'main' into timepickerChecksUrl

61056f4

dmlemeshko merged commit 1734b0e into elastic:main Dec 4, 2022

kibanamachine added the v8.7.0 label Dec 4, 2022

kibanamachine mentioned this pull request Dec 4, 2022

[8.6] quick check of current URL to skip timepicker (#146462) #146954

Merged

kibanamachine added the v8.6.0 label Dec 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

quick check of current URL to skip timepicker #146462

quick check of current URL to skip timepicker #146462

LeeDr commented Nov 28, 2022 •

edited by kibanamachine

Loading

LeeDr commented Nov 28, 2022 •

edited

Loading

LeeDr commented Nov 30, 2022

LeeDr Nov 30, 2022

dmlemeshko Nov 30, 2022

LeeDr Nov 30, 2022

dmlemeshko Nov 30, 2022

wayneseymour Nov 30, 2022

LeeDr Nov 30, 2022

elasticmachine commented Nov 30, 2022

dmlemeshko commented Dec 2, 2022

dmlemeshko commented Dec 2, 2022

drewdaemon Dec 2, 2022

wayneseymour Dec 2, 2022

dmlemeshko Dec 2, 2022

stratoula Dec 2, 2022

dmlemeshko commented Dec 4, 2022

kibana-ci commented Dec 4, 2022

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

kibanamachine commented Dec 4, 2022

kibanamachine commented Dec 4, 2022

quick check of current URL to skip timepicker #146462

quick check of current URL to skip timepicker #146462

Conversation

LeeDr commented Nov 28, 2022 • edited by kibanamachine Loading

Summary

LeeDr commented Nov 28, 2022 • edited Loading

LeeDr commented Nov 30, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 30, 2022

dmlemeshko commented Dec 2, 2022

dmlemeshko commented Dec 2, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dmlemeshko commented Dec 4, 2022

kibana-ci commented Dec 4, 2022

💚 Build Succeeded

Metrics [docs]

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

kibanamachine commented Dec 4, 2022

kibanamachine commented Dec 4, 2022

💚 All backports created successfully

Questions ?

LeeDr commented Nov 28, 2022 •

edited by kibanamachine

Loading

LeeDr commented Nov 28, 2022 •

edited

Loading