Failing test: Jest Tests.src/plugins/vis_type_vega/public by alexwizp · Pull Request #71834 · elastic/kibana

alexwizp · 2020-07-15T09:43:27Z

Summary

A test failed on a tracked branch

TypeError: Cannot read property 'height' of null
    at Object.test (/dev/shm/workspace/kibana/src/plugins/vis_type_vega/public/vega_visualization.test.js:226:25)
    at process._tickCallback (internal/process/next_tick.js:68:7)

First failure: Jenkins Build

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
This was checked for keyboard-only and screenreader accessibility
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

alexwizp · 2020-07-15T09:46:02Z

src/plugins/vis_type_vega/public/vega_visualization.test.js

-
-        await vegaVis.render(vegaParser);
-        const vegaView = vegaVis._vegaView._view;
-        expect(vegaView.height()).toBe(250.00000001);


I think problem was in that line. I decide to remove that test cause we already have have some snapshots with that adjusted height

alexwizp · 2020-07-15T11:41:08Z

@elasticmachine merge upstream

alexwizp · 2020-07-15T13:18:39Z

@elasticmachine merge upstream

alexwizp · 2020-07-15T15:16:55Z

@elasticmachine merge upstream

alexwizp · 2020-07-16T07:59:04Z

@elasticmachine merge upstream

alexwizp · 2020-07-16T13:44:02Z

@elasticmachine merge upstream

alexwizp · 2020-07-16T14:28:41Z

@elasticmachine merge upstream

alexwizp · 2020-07-16T15:43:21Z

@elasticmachine merge upstream

alexwizp · 2020-07-16T18:21:51Z

@elasticmachine merge upstream

alexwizp · 2020-07-17T07:49:50Z

@elasticmachine merge upstream

flash1293

I think problem was in that line.

Quite possible. Actually, while we are at it - I introduced this weird hack a while back because the height signal would not work right when the height of the canvas is an even number.
https://github.com/flash1293/kibana/blob/97d81288bd1d3e19e8dece12c08d1cb9dbe1203a/src/plugins/vis_type_vega/public/vega_view/vega_base_view.js#L200

As we updated to vega 5 lately it's very possible this bug got fixed. If this is. the case, let's not just remove the test but the workaround itself as well. Could you test whether that's the case?

The test spec deleted in this PR is a good way to validate:
https://github.com/elastic/kibana/pull/71834/files#diff-ac66b0b6d3fe13ce5d2bc22b202f7450L194-L213

This should display a centered "Test" text. When slowly changing the height of the browser window, the text should stay centered - if the bug is still present in vega, it will jump to the top and back to the center.

alexwizp · 2020-07-22T11:48:46Z

@elasticmachine merge upstream

Related to elastic#31461

alexwizp · 2020-07-22T12:46:13Z

#31461 fix was removed. All works fine with new version of Vega

elasticmachine · 2020-07-22T12:48:09Z

Pinging @elastic/kibana-app (Team:KibanaApp)

elasticmachine · 2020-07-22T12:48:41Z

Pinging @elastic/kibana-test-triage (failed-test)

kibanamachine · 2020-07-22T14:24:43Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: bd26897

Build metrics

async chunks size

id	value	diff	baseline
visTypeVega	1.4MB	-47.0B	1.4MB

History

💚 Build #63567 succeeded d56a0fa
💚 Build #62547 succeeded 426a6ba
💚 Build #62385 succeeded fbb20da
💚 Build #62317 succeeded b9d0533
💚 Build #62272 succeeded 8af777e

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

flash1293

Seen it works, code LGTM

) * Failing test: Jest Tests.src/plugins/vis_type_vega/public * remove workaround for vega height bug Related to elastic#31461 Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

* master: (35 commits) Migrated karma tests to jest (elastic#72649) Migrate status page app to core (elastic#72017) Failing test: Jest Tests.src/plugins/vis_type_vega/public (elastic#71834) Fix Firefox TSVB flaky test with switch index patterns (elastic#72882) [ML] Fixing link to index management from file data visualizer (elastic#72863) test: 💍 add test for sub-expression variables (elastic#71644) fix bug (elastic#72809) [keystore] use get_keystore in server cli (elastic#72954) Show step number instead of incomplete step. (elastic#72866) Fix bug where user can't add an exception when "close alert" is checked (elastic#72919) [Monitoring] Fix issues displaying alerts (elastic#72891) [Ingest Manager] Add more Fleet concurrency tests elastic#71744 (elastic#72338) [Security Solution][Exceptions] - Update UI exceptions builder nested logic (elastic#72490) disable renovate masterIssue [ML] API integration tests for UPDATE data frame analytics endpoint (elastic#72710) [Uptime] Fix accessibility issue in Uptime app nav links (elastic#72926) [Maps] fix removing global filter from layer can cause app to start thrashing (elastic#72763) [Maps] fix blended layer aggregation error when using composite aggregation (elastic#72759) fix unexpected arguments to unload command Limits the upload size of lists to 9 meg size (elastic#72898) ...

…73015) * Failing test: Jest Tests.src/plugins/vis_type_vega/public * remove workaround for vega height bug Related to #31461 Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Failing test: Jest Tests.src/plugins/vis_type_vega/public

9d8bf8c

alexwizp commented Jul 15, 2020

View reviewed changes

alexwizp requested a review from DianaDerevyankina July 15, 2020 09:46

DianaDerevyankina approved these changes Jul 15, 2020

View reviewed changes

Merge branch 'master' into vega_test

c7097ab

Merge branch 'master' into vega_test

ab4e24b

Merge branch 'master' into vega_test

b69f296

Merge branch 'master' into vega_test

931bb7e

Merge branch 'master' into vega_test

c2b15b2

Merge branch 'master' into vega_test

8af777e

Merge branch 'master' into vega_test

b9d0533

Merge branch 'master' into vega_test

fbb20da

Merge branch 'master' into vega_test

426a6ba

alexwizp requested a review from flash1293 July 17, 2020 09:57

alexwizp self-assigned this Jul 17, 2020

alexwizp marked this pull request as ready for review July 17, 2020 09:57

alexwizp requested a review from a team July 17, 2020 09:57

flash1293 reviewed Jul 17, 2020

View reviewed changes

elasticmachine and others added 2 commits July 22, 2020 05:48

Merge branch 'master' into vega_test

d56a0fa

remove workaround for vega height bug

bd26897

Related to elastic#31461

alexwizp added v7.10.0 v8.0.0 Feature:Vega Vega visualizations Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// labels Jul 22, 2020

alexwizp added release_note:skip Skip the PR/issue when compiling release notes failed-test A test failure on a tracked branch, potentially flaky-test labels Jul 22, 2020

flash1293 approved these changes Jul 23, 2020

View reviewed changes

alexwizp merged commit b2a473d into elastic:master Jul 23, 2020

alexwizp mentioned this pull request Jul 23, 2020

[7.x] Failing test: Jest Tests.src/plugins/vis_type_vega/public (#71834) #73015

Merged

Conversation

alexwizp commented Jul 15, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

For maintainers

Uh oh!

alexwizp Jul 15, 2020

Choose a reason for hiding this comment

Uh oh!

alexwizp commented Jul 15, 2020

Uh oh!

alexwizp commented Jul 15, 2020

Uh oh!

alexwizp commented Jul 15, 2020

Uh oh!

alexwizp commented Jul 16, 2020

Uh oh!

alexwizp commented Jul 16, 2020

Uh oh!

alexwizp commented Jul 16, 2020

Uh oh!

alexwizp commented Jul 16, 2020

Uh oh!

alexwizp commented Jul 16, 2020

Uh oh!

alexwizp commented Jul 17, 2020

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

alexwizp commented Jul 22, 2020

Uh oh!

alexwizp commented Jul 22, 2020

Uh oh!

elasticmachine commented Jul 22, 2020

Uh oh!

elasticmachine commented Jul 22, 2020

Uh oh!

kibanamachine commented Jul 22, 2020

💚 Build Succeeded

Build metrics

async chunks size

History

Uh oh!

flash1293 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alexwizp commented Jul 15, 2020 •

edited

Loading