New scripted field tests #9242

LeeDr · 2016-11-29T14:51:04Z

As the comments in the new test/functional/apps/management/_scripted_fields.js describe, 12 new tests for scripted fields.
5 scripted fields are created;

expression
painless number
painless string
painless boolean
painless date

In each case we check that;

we create the scripted field
the script produces the expected result in the doc view of discover
that we can filter by a value of the scripted field
that we can click the Visualize button for the scripted field and get the expected results

The tests can't currently sort by the scripted fields in discover because the sort arrow doesn't show up until you mouse over and we don't have that functionality in our tests yet.
We also can't drag over a chart to filter.
We could probably click an element in a chart to filter, but none of our tests have tried that yet.

Closes #8594

…ripted fields

LeeDr · 2016-12-09T15:07:29Z

Test failed so it's back on me...

LeeDr · 2016-12-09T17:55:52Z

Setting the timezone to UTC and fixing the expected results to that fixed the previous failures.

LeeDr · 2016-12-09T22:34:11Z

These tests do not currently sort by scripted fields. Are we talking about on the Discover tab in the doc list?

Bargs · 2016-12-09T23:28:50Z

Yep, on discover or a saved search in a dashboard I think

Bargs · 2016-12-12T19:58:32Z

@LeeDr the first time I ran the tests I got one error: https://gist.github.com/Bargs/4e63087c383d26bace3bc88168bea41e

The second time I ran them everything passed. I wonder if there's some timing issue in that test?

Bargs

This looks like a great start for scripted field tests. I left some minor comments but for the most part the code seems to be in good shape!

I'll make some suggestions for additional things we could test. I don't think there's any reason to hold up this PR, I'm just thinking out loud for the future.

Date and boolean type painless fields. These could really benefit from testing since they're brand new. There was already one bug with filtering on booleans.
Sorting by all scripted field types on Discover
Adding filters on scripted fields by clicking on the charts in Visualize
Adding a range filter by brushing on a histogram based on scripted field
Test scripted fields in each of the different agg types. Different aggs use scripts differently, for instance significant terms doesn't support them at all
Ensure that only painless and expression languages are available once Limit scripted fields to painless and expression langs #9172 is merged
Ensure chart labels are built correctly for scripted fields

Bargs · 2016-12-12T20:09:44Z

test/functional/apps/management/_scripted_fields.js

+    await PageObjects.visualize.waitForVisualization();
+    await PageObjects.visualize.collapseChart();
+    await PageObjects.settings.setPageSize('All');
+    var data = await PageObjects.visualize.getDataTableData();


All vars should be updated to const or let

Bargs · 2016-12-12T20:12:19Z

test/functional/apps/management/_scripted_fields.js

+
+bdd.describe('creating and using Painless numeric scripted fields', function describeIndexTests() {
+
+  const scriptedExpressionFieldName = 'ram_expr1';


Unused const

Bargs · 2016-12-12T20:34:26Z

test/support/page_objects/discover_page.js

+  clickFieldListItemVisualize(field) {
+    return this.findTimeout
+    .findByCssSelector('li[attr-field="' + field + '"] > a').click();
+  }


This and the method above feel a little too generic, I'd worry about them breaking if we add another link or button. Maybe add data-test-subj attributes for these too?

I didn't fix this. Yes, I agree we need a lot more data-test-subjs. I'd like to get these tests in and have another PR for adding data-test-subjs and using them in the tests.

Bargs · 2016-12-12T20:35:13Z

test/support/page_objects/discover_page.js

+  disableFilter(field) {
+    return PageObjects.common.findTestSubject('disableFilter-' + field)
+    .click();
+  }


Can we just remove these if they don't work?

Bargs · 2016-12-12T20:36:06Z

test/support/page_objects/discover_page.js

+    .click()
+    .then(() => {
+      return this.findTimeout
+      .findByCssSelector('a[ng-click="removeAll()"]')


I wouldn't rely on ng-click values, these could very easily change in the future.

I'd like to get these tests in and have another PR for adding data-test-subjs and using them in the tests.

Bargs · 2016-12-12T20:48:10Z

test/support/page_objects/settings_page.js

+  setScriptedFieldScript(script) {
+    PageObjects.common.debug('set scripted field script = ' + script);
+    return this.remote.setFindTimeout(defaultFindTimeout)
+    .findByCssSelector('textarea[ng-model="editor.field.script"]')


As with ng-click, I'm worried about relying on ng-model in the selectors here. A simple change in the angular code could break these.

I'd like to get these tests in and have another PR for adding data-test-subjs and using them in the tests.

Bargs · 2016-12-12T20:50:17Z

test/support/page_objects/visualize_page.js

@@ -565,12 +565,12 @@ export default class VisualizePage {
    .then(function getRect() {
      return self
      .setFindTimeout(defaultFindTimeout)
-      .findByCssSelector('rect.background')
+      .findByCssSelector('div.chart-wrapper > div.chart > svg')


Not sure what this change is for? This method doesn't appear to be used in the scripted field tests.

I reverted that change. Not sure what made me make the initial change. Must have been a problem when running the whole test suite. But it passed for me now without that change.

LeeDr · 2016-12-13T21:09:31Z

I'm going to try adding sort to the tests. Meanwhile I'll let jenkins have another pass at them as-is.

tbragin · 2016-12-14T05:21:46Z

Thanks for working on this @LeeDr and @Bargs !

LeeDr · 2016-12-15T20:17:08Z

@Bargs I think I know what you had a test failure at one point. Most of our tests delete the .kibana index and let Kibana re-create it, and then we also set the timezone to UTC by updating the config doc directly. But depending on what you do next in Kibana (like create a new index pattern) it can write it's config doc again and clobber the timezone setting we wrote. So I added an after method to the _kibana_settings test suite to set it back to UTC when those tests are done. It doesn't really solve the issue, but works.

LeeDr · 2016-12-16T17:30:39Z

FYI, test output;

>> PASS: chrome on any platform - kibana - settings app - creating and using Lucence expression scripted fields - should create scripted field (4981ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Lucence expression scripted fields - should see scripted field value in Discover (9050ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Lucence expression scripted fields - should filter by scripted field value in Discover (865ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Lucence expression scripted fields - should visualize scripted field in vertical bar chart (1625ms)
>> 0/4 tests failed

>> PASS: chrome on any platform - kibana - settings app - creating and using Painless numeric scripted fields - should create scripted field (4849ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless numeric scripted fields - should see scripted field value in Discover (8710ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless numeric scripted fields - should filter by scripted field value in Discover (954ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless numeric scripted fields - should visualize scripted field in vertical bar chart (1333ms)
>> 0/4 tests failed

>> PASS: chrome on any platform - kibana - settings app - creating and using Painless string scripted fields - should create scripted field (5092ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless string scripted fields - should see scripted field value in Discover (8658ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless string scripted fields - should filter by scripted field value in Discover (1047ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless string scripted fields - should visualize scripted field in vertical bar chart (895ms)
>> 0/4 tests failed

>> PASS: chrome on any platform - kibana - settings app - creating and using Painless boolean scripted fields - should create scripted field (4955ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless boolean scripted fields - should see scripted field value in Discover (8797ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless boolean scripted fields - should filter by scripted field value in Discover (2399ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless boolean scripted fields - should visualize scripted field in vertical bar chart (1019ms)
>> 0/4 tests failed

>> PASS: chrome on any platform - kibana - settings app - creating and using Painless date scripted fields - should create scripted field (5487ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless date scripted fields - should see scripted field value in Discover (6812ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless date scripted fields - should filter by scripted field value in Discover (650ms)
>> PASS: chrome on any platform - kibana - settings app - creating and using Painless date scripted fields - should visualize scripted field in vertical bar chart (898ms)
>> 0/4 tests failed

Bargs · 2016-12-20T18:07:19Z

LGTM. One small request. In the future, when you want to make a big refactoring like this change to async/await could you do that in a separate PR or at least a separate commit? All of the additional noise makes it really difficult to verify the intended fix. And since there's always a chance a refactoring could break something, it adds a lot of unrelated code to review.

Backports PR #9242 **Commit 1:** [WIP] new scripted field tests * Original sha: 6440101 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-11-29T14:33:58Z **Commit 2:** Merge branch 'master' into scriptedFieldTests * Original sha: 4590fcb * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-08T19:39:57Z **Commit 3:** Final improvements on 12 new tests for 1 expression and 2 painless scripted fields * Original sha: 7e85c64 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-09T04:48:45Z **Commit 4:** Add try loops around testing first Discover doc * Original sha: 8dfab84 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-09T15:50:50Z **Commit 5:** Set timezone to UTC and adjusted data accordingly * Original sha: 78018ad * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-09T17:23:31Z **Commit 6:** Merge branch 'master' into scriptedFieldTests * Original sha: 75eb350 * Authored by Spencer <spalger@users.noreply.github.com> on 2016-12-13T18:13:54Z * Committed by GitHub <noreply@github.com> on 2016-12-13T18:13:54Z **Commit 7:** Added boolean and date Painless scripted field types * Original sha: 1854ee3 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-15T19:59:04Z **Commit 8:** Remove unused (non-working) methods * Original sha: c190cc5 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-15T20:07:15Z **Commit 9:** Merge branch 'master' into scriptedFieldTests * Original sha: ca29b91 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-15T20:08:47Z **Commit 10:** Fix lint error * Original sha: c4f3e6c * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-15T20:18:49Z **Commit 11:** Added several data-test-subj attributes and used them in the tests * Original sha: 0528a55 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-16T22:28:01Z **Commit 12:** Reverting previous change to getBarChartData * Original sha: 047d912 * Authored by LeeDr <lee.drengenberg@elastic.co> on 2016-12-20T15:01:27Z

* new scripted field tests * Final improvements on 12 new tests for 1 expression and 2 painless scripted fields * Add try loops around testing first Discover doc * Set timezone to UTC and adjusted data accordingly * Added boolean and date Painless scripted field types * Remove unused (non-working) methods * Fix lint error * Added several data-test-subj attributes and used them in the tests * Reverting previous change to getBarChartData

[WIP] new scripted field tests

6440101

LeeDr added the test label Nov 29, 2016

tbragin added the Feature:Scripted Fields Scripted fields features label Nov 30, 2016

LeeDr added 2 commits December 8, 2016 13:39

Merge branch 'master' into scriptedFieldTests

4590fcb

Final improvements on 12 new tests for 1 expression and 2 painless sc…

7e85c64

…ripted fields

LeeDr changed the title ~~[WIP] new scripted field tests~~ New scripted field tests Dec 9, 2016

LeeDr added the review label Dec 9, 2016

LeeDr assigned Bargs Dec 9, 2016

LeeDr removed the review label Dec 9, 2016

LeeDr added 2 commits December 9, 2016 09:50

Add try loops around testing first Discover doc

8dfab84

Set timezone to UTC and adjusted data accordingly

78018ad

LeeDr added the review label Dec 9, 2016

Bargs self-requested a review December 9, 2016 18:26

LeeDr assigned tbragin Dec 9, 2016

Bargs suggested changes Dec 12, 2016

View reviewed changes

Bargs added updates_needed and removed review labels Dec 13, 2016

Merge branch 'master' into scriptedFieldTests

75eb350

LeeDr added 3 commits December 15, 2016 13:59

Added boolean and date Painless scripted field types

1854ee3

Remove unused (non-working) methods

c190cc5

Merge branch 'master' into scriptedFieldTests

ca29b91

Fix lint error

c4f3e6c

LeeDr added review and removed updates_needed labels Dec 15, 2016

LeeDr added v5.2.0 v6.0.0 labels Dec 16, 2016

LeeDr added 2 commits December 16, 2016 16:28

Added several data-test-subj attributes and used them in the tests

0528a55

Reverting previous change to getBarChartData

047d912

Bargs approved these changes Dec 20, 2016

View reviewed changes

LeeDr merged commit 0cbec2c into elastic:master Dec 20, 2016

elastic-jasper mentioned this pull request Dec 20, 2016

[5.x] New scripted field tests #9574

Closed

LeeDr mentioned this pull request Jan 2, 2017

5.x backport #9242 New scripted field tests #9700

Merged

LeeDr deleted the scriptedFieldTests branch May 23, 2017 13:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New scripted field tests #9242

New scripted field tests #9242

LeeDr commented Nov 29, 2016 •

edited

Loading

LeeDr commented Dec 9, 2016

LeeDr commented Dec 9, 2016

LeeDr commented Dec 9, 2016

Bargs commented Dec 9, 2016

Bargs commented Dec 12, 2016

Bargs left a comment

Bargs Dec 12, 2016

Bargs Dec 12, 2016

LeeDr Dec 15, 2016

Bargs Dec 12, 2016

LeeDr Dec 15, 2016

Bargs Dec 12, 2016

Bargs Dec 12, 2016

LeeDr Dec 15, 2016

Bargs Dec 12, 2016

LeeDr Dec 15, 2016

Bargs Dec 12, 2016

LeeDr Dec 20, 2016

LeeDr commented Dec 13, 2016

tbragin commented Dec 14, 2016

LeeDr commented Dec 15, 2016

LeeDr commented Dec 16, 2016

Bargs commented Dec 20, 2016


		bdd.describe('creating and using Painless numeric scripted fields', function describeIndexTests() {

		const scriptedExpressionFieldName = 'ram_expr1';

New scripted field tests #9242

New scripted field tests #9242

Conversation

LeeDr commented Nov 29, 2016 • edited Loading

LeeDr commented Dec 9, 2016

LeeDr commented Dec 9, 2016

LeeDr commented Dec 9, 2016

Bargs commented Dec 9, 2016

Bargs commented Dec 12, 2016

Bargs left a 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LeeDr commented Dec 13, 2016

tbragin commented Dec 14, 2016

LeeDr commented Dec 15, 2016

LeeDr commented Dec 16, 2016

Bargs commented Dec 20, 2016

LeeDr commented Nov 29, 2016 •

edited

Loading