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

[ML] Functional Tests: Extend canvas assertion options. #91473

Merged
merged 41 commits into from
Mar 19, 2021

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Feb 16, 2021

Summary

Fixes #91450.
Fixes #94854.

Updates the canvas element assertion service:

  • Expected colors can now be filtered, useful for example to exclude a known dominant background color.
  • Colors themselves will now be asserted with a given tolerance for each RGB channel.
  • Anti-aliasing can be turned off to minimize differences in font rendering across systems.
  • isColorWithinTolerance() now exposed as part of the canvas element service to make it available for single color checks.

These options in combination should make assertions more stable.

Checklist

For maintainers

@walterra walterra added :ml test_xpack_functional v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Feb 16, 2021
@walterra walterra requested a review from pheyos February 16, 2021 12:25
@walterra walterra self-assigned this Feb 16, 2021
@walterra walterra requested a review from a team as a code owner February 16, 2021 12:25
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra
Copy link
Contributor Author

Further investigating the test failures, the tests are now set up to scroll to the scatterplot before asserting the canvas element, here's a screenshot of the failed test, need to compare against local results next:
image

@walterra walterra requested a review from a team as a code owner February 24, 2021 09:56
@walterra
Copy link
Contributor Author

@elasticmachine merge upstream

@walterra walterra added v7.12.1 and removed v7.12.0 labels Mar 17, 2021
@walterra walterra requested a review from darnautov March 17, 2021 15:31
@walterra
Copy link
Contributor Author

@darnautov @peteharverson yay, this passed finally, ready for review :)

{ key: '#DDDDDD', value: 3 },
// line
{ key: '#6092C0', value: 1 },
{ key: '#DDDDDD', value: 48 },
Copy link
Contributor

@peteharverson peteharverson Mar 17, 2021

Choose a reason for hiding this comment

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

Running locally on Windows, I had to edit these values to the following to get it to pass:

            // tick/grid/axis
            { key: '#DDDDDD', value: 50 },
            // lines
            { key: '#98A2B3', value: 30 },
            { key: '#6092C0', value: 10 },
            { key: '#5F92C0', value: 6 },

Before that, this single test was failing with the error

      retry.tryForTime timeout: Error: Color stats for 'mlDFAnalyticsClassificationExplorationRocCurveChart' should be w
ithin tolerance. Expected: '[{"key":"#DDDDDD","value":48},{"key":"#98A2B3","value":32},{"key":"#6092C0","value":10},{"ke
y":"#5F92C0","value":6}]' (got '[{"key":"#DDDDDD","value":54.5920820644334,"withinTolerance":false},{"key":"#98A2B3","va
lue":26.106747876262215,"withinTolerance":false},{"key":"#6092C0","value":9.145696425709247,"withinTolerance":true},{"ke
y":"#5F92C0","value":9.145696425709247,"withinTolerance":true}]')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, updated in b70f3f9, let's see if it's within the tolerance to still pass CI :)

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested locally and LGTM

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
ml 6.4MB 6.4MB +786.0B

History

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

cc @walterra

@walterra
Copy link
Contributor Author

Yay, flaky test runner passed 42x without failures. https://kibana-ci.elastic.co/job/kibana+flaky-test-suite-runner/1384/

@walterra walterra merged commit 778bb69 into elastic:master Mar 19, 2021
@walterra walterra deleted the ml-fix-canvas-element branch March 19, 2021 20:07
walterra added a commit to walterra/kibana that referenced this pull request Mar 19, 2021
Updates the canvas element assertion service and stabilizes tests.
# Conflicts:
#	x-pack/test/functional/apps/ml/data_frame_analytics/outlier_detection_creation.ts
@walterra walterra mentioned this pull request Mar 19, 2021
15 tasks
walterra added a commit that referenced this pull request Mar 19, 2021
)

Updates the canvas element assertion service and stabilizes tests.
# Conflicts:
#	x-pack/test/functional/apps/ml/data_frame_analytics/outlier_detection_creation.ts
walterra added a commit to walterra/kibana that referenced this pull request Apr 6, 2021
Updates the canvas element assertion service and stabilizes tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment