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.

Sign up for GitHub

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

Jump to bottom

add alternate path for x-pack/Cloud test for Lens #82634

Merged
merged 3 commits into from
Nov 9, 2020
Merged

add alternate path for x-pack/Cloud test for Lens #82634

merged 3 commits into from
Nov 9, 2020

Conversation

LeeDr
Copy link
Contributor

@LeeDr LeeDr commented Nov 4, 2020

Summary

A set of OS management scripted field tests click the Visualize button in Discover and then expect to use the Inspect menu item to verify the data in the OSS Vertical Bar Chart visualization. But when these tests are run against Cloud or any default distribution of Kibana they fail.
This PR adds a check for isOss() in those tests and when not OSS, it checks that the expected name of the aggregation of the scripted field is in the lens data-test-subj. So it's only verifying that Lens opened and the aggregation name is there, not that the visualization actually rendered. I don't think we have a good way to verify the whole visualization rendered right now other than a screenshot comparison test.

Another possible way to attack this OSS vs Default distribution issue in the tests would be to copy these tests from OSS to x-pack, add skipCloud tag to the OSS ones, and have the x-pack tests always check Lens. But this adds pretty much redundancy to UI functional tests in CI. The current way keeps the OSS test in CI and the x-pack path on Cloud jobs.

Checklist

Delete any items that are not applicable to this PR.

@LeeDr LeeDr added Feature:Scripted Fields Scripted fields features Feature:Discover Discover Application test_ui_functional Team:Visualizations Visualization editors, elastic-charts and infrastructure Feature:Lens Feature:Functional Testing labels Nov 4, 2020
@elasticmachine
Copy link
Contributor

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

@LeeDr LeeDr requested a review from liza-mae November 4, 2020 17:18
@LeeDr LeeDr added release_note:skip Skip the PR/issue when compiling release notes v7.10.0 v7.11.0 v8.0.0 labels Nov 4, 2020
@LeeDr
Copy link
Contributor Author

LeeDr commented Nov 4, 2020

fixes: #80239

@LeeDr
Copy link
Contributor Author

LeeDr commented Nov 4, 2020

@elasticmachine merge upstream

flash1293
flash1293 approved these changes Nov 5, 2020
View reviewed changes
Copy link
Contributor

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

liza-mae
liza-mae reviewed Nov 5, 2020
View reviewed changes
Copy link
Contributor

@liza-mae liza-mae left a comment

Choose a reason for hiding this comment

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

I recommend this to be tested on cloud flaky test runner to make sure it passes.
https://internal-ci.elastic.co/view/Stack%20Tests/job/elastic+estf-cloud-kibana-flaky-test-runner/build?delay=0sec

@flash1293
Copy link
Contributor

flash1293 commented Nov 5, 2020 •
edited
Loading

The whole setup is a little brittle because it's not using the lens page object as this is OSS code. The alternative would be to move the lens conditional branches here to a separate test with a similar setup in x-pack and just skip those OSS tests if we are running with x-pack enabled.

I think it's fine for this one use case though, if we run into the issue having to validate Lens stuff in OSS tests more often we should think about a more stable solution.

@dmlemeshko dmlemeshko self-requested a review November 5, 2020 19:46
dmlemeshko
dmlemeshko approved these changes Nov 5, 2020
View reviewed changes
Copy link
Member

@dmlemeshko dmlemeshko left a comment

Choose a reason for hiding this comment

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

LGTM

@LeeDr
Copy link
Contributor Author

LeeDr commented Nov 5, 2020

@flash1293 yes, when I went to fix this test I quickly realized that the lens page object isn't in the OSS test area. I thought about making a copy of the test in x-pack, and about skipCloud on the OSS one.

@LeeDr
Copy link
Contributor Author

LeeDr commented Nov 9, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@LeeDr
Copy link
Contributor Author

LeeDr commented Nov 9, 2020 •
edited
Loading

@liza-mae 40 runs passed on Jenkins
Stack Tests
elastic / estf-cloud-kibana-flaky-test-runner
167 - saas - LeeDr - fixCloudMgmtLens

Oh, but this PR skips the test on Cloud anyway!

@LeeDr LeeDr merged commit 83e51f5 into elastic:master Nov 9, 2020
@LeeDr LeeDr deleted the fixCloudMgmtLens branch November 9, 2020 17:46
LeeDr pushed a commit to LeeDr/kibana that referenced this pull request Nov 9, 2020
@kibanamachine
add alternate path for x-pack/Cloud test for Lens (elastic#82634)
bad3036 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@LeeDr LeeDr mentioned this pull request Nov 9, 2020
Merged
LeeDr pushed a commit to LeeDr/kibana that referenced this pull request Nov 9, 2020
@kibanamachine
add alternate path for x-pack/Cloud test for Lens (elastic#82634)
00f8e89 
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@LeeDr LeeDr mentioned this pull request Nov 9, 2020
Merged
LeeDr pushed a commit that referenced this pull request Nov 9, 2020
@kibanamachine
[7.x] add alternate path for x-pack/Cloud test for Lens (#82634) (#82993
68446d7 
)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
LeeDr pushed a commit that referenced this pull request Nov 9, 2020
@kibanamachine
[7.10] add alternate path for x-pack/Cloud test for Lens (#82634) (#8…
603cb28 
…2994)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 10, 2020
@gmmorris
Merge branch 'master' into alerts/stack-alerts-public
aadbe35 
* master: (39 commits)
  Fix ilm navigation (elastic#81664)
  [Lens] Distinct icons for XY and pie chart value labels toolbar (elastic#82927)
  [data.search.aggs] Throw an error when trying to create an agg type that doesn't exist. (elastic#81509)
  Index patterns api - load field list on server (elastic#82629)
  New events resolver (elastic#82170)
  [App Search] Misc naming tech debt (elastic#82770)
  load empty_kibana in test to have clean starting point (elastic#82772)
  Remove data <--> expressions circular dependencies. (elastic#82685)
  Update 8.0 breaking change template to gather information on how to programmatically detect it. (elastic#82905)
  Add alerting as codeowners to related documentation folder (elastic#82777)
  Add captions to user and space grid pages (elastic#82713)
  add alternate path for x-pack/Cloud test for Lens (elastic#82634)
  Uses asCurrentUser in getClusterUuid (elastic#82908)
  [Alerting][Connectors] Add new executor subaction to get 3rd party case fields (elastic#82519)
  Fix test import objects (elastic#82767)
  [ML] Add option for anomaly charts for metric detector should plot min, mean or max as appropriate (elastic#81662)
  Update alert type selection layout to rows instead of grid (elastic#73665)
  Prevent Kerberos and PKI providers from initiating a new session for unauthenticated XHR/API requests. (elastic#82817)
  Update grunt and related packages (elastic#79327)
  Allow the repository to search across all namespaces (elastic#82863)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liza-mae liza-mae liza-mae left review comments

@flash1293 flash1293 flash1293 approved these changes

@dmlemeshko dmlemeshko dmlemeshko approved these changes

@stratoula stratoula Awaiting requested review from stratoula

Assignees
No one assigned
Labels
Feature:Discover Discover Application Feature:Functional Testing Feature:Lens Feature:Scripted Fields Scripted fields features release_note:skip Skip the PR/issue when compiling release notes Team:Visualizations Visualization editors, elastic-charts and infrastructure test_ui_functional v7.10.0 v7.11.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@LeeDr @elasticmachine @flash1293 @kibanamachine @dmlemeshko @liza-mae