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

[Graph] Re-enable functional test #44683

Merged
merged 51 commits into from
Sep 10, 2019

Conversation

flash1293
Copy link
Contributor

@flash1293 flash1293 commented Sep 3, 2019

Summary

Depends on #44261

Fixes #18084

Re-enables functional tests for Graph. There were some problems outlined in the issue with testing the shown nodes and edges because the layouting algorithm is not deterministic.

This PR tries to work around these issues and resolves breakages to have coverage for the happy path. This is expected to be temporary solution, because the workspace will be refactored soon (see #44225) and I will make sure to make the visualization accessible which will also simplify functional tests.

Changes:

  • Match nodes and edges by their x/y positions. To do so, stop the layout before fetching all the elements and matching them up (assigning a source and a target to each edge element)
  • Clicking an edge is difficult because it can be shadowed by a lot of other stuff. This is solved by removing all nodes but source and target node, then stopping the layout and clicking the center of the line element
  • When comparing line widths, take an epsilon into account to be robust with regard to floating point issues
  • Don't validate the geometry of the venn diagram, the values from the labels are sufficient IMHO - the venn diagram rendering logic can be covered by unit tests (also on the roadmap issue [Meta] Graph Technical Plan #44225)

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@flash1293 flash1293 requested review from kertal and LeeDr and removed request for a team September 5, 2019 18:02
@flash1293 flash1293 added release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.5.0 v8.0.0 labels Sep 5, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app

@elasticmachine
Copy link
Contributor

💔 Build Failed

@flash1293
Copy link
Contributor Author

Seems like clicking the edge is not stable yet - I will look into this.

@flash1293
Copy link
Contributor Author

OK, selecting the edge should be stable now - ran it ~20 times locally without failures.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

just a quick have-a-good-weekend review, started the tests 10x, all passing, look at the details on monday

Copy link
Contributor

@LeeDr LeeDr left a comment

Choose a reason for hiding this comment

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

I haven't pulled this PR yet to run it yet. Just wanted to see if you considered this approach vs hiding the other nodes.

x-pack/test/functional/apps/graph/graph.ts Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Code LGTM, run the tests several times, no problems, here's the award for no-flakiness 🥇

x-pack/test/functional/page_objects/graph_page.ts Outdated Show resolved Hide resolved
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@LeeDr LeeDr left a comment

Choose a reason for hiding this comment

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

LGTM - I pulled and ran it locally. Looks good!

@flash1293 flash1293 merged commit c46eea5 into elastic:master Sep 10, 2019
@flash1293 flash1293 deleted the graph/functional-test branch September 10, 2019 08:20
flash1293 added a commit to flash1293/kibana that referenced this pull request Sep 10, 2019
jloleysens added a commit to jloleysens/kibana that referenced this pull request Sep 10, 2019
…-to-np-ready

* 'master' of github.com:elastic/kibana: (138 commits)
  [Canvas] i18n work on workpad header (and a few header CTAs) and convert to typescript (elastic#44943)
  update close/delete system index modals (elastic#45037)
  TS return type of createIndexPatternSelect (elastic#45107)
  [ML] Fix focus chart updating. (elastic#45146)
  [ML] Data frame transform: Fix progress in wizard create step. (elastic#45116)
  [Graph] Re-enable functional test (elastic#44683)
  [SIEM] unique table id for each top talkers table (elastic#45014)
  [SIEM] ip details heading draggable (elastic#45179)
  [Maps][File upload] Set complete on index pattern creation (elastic#44423)
  [Maps] unmount map embeddable component on destroy (elastic#45183)
  [SIEM] Adds error toasts to MapEmbeddable component (elastic#45088)
  fix redirect to maintain search query string (elastic#45184)
  [APM] One-line trace summary (elastic#44842)
  [Infra UI] Display non-metric details on Node Detail page (elastic#43551)
  [Maps][File upload] Removing bbox from parsed file pending upstream lib fix (elastic#45194)
  [Logs UI] Improve live streaming behavior when scrolling (elastic#44923)
  [APM] Fix indefinite loading state in agent settings for unauthorized user roles (elastic#44970)
  [Reporting] Rewrite addForceNowQuerystring to getFullUrls (elastic#44851)
  [Reporting/ESQueue] Improve logging of doc-update events (elastic#45077)
  [Reporting] Make screenshot capture less noisy by default (elastic#45185)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Graph Graph application feature release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure v7.5.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Graph] Re-enable Functional Tests
4 participants