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

[Archive Migration] kibana_sample_data_flights_index_pattern #127230

Merged
merged 7 commits into from
Mar 16, 2022
Merged

[Archive Migration] kibana_sample_data_flights_index_pattern #127230

merged 7 commits into from
Mar 16, 2022

Conversation

LeeDr
Copy link

@LeeDr LeeDr commented Mar 8, 2022

Summary

Unskip and Migrate from 'test/functional/fixtures/es_archiver/kibana_sample_data_flights_index_pattern' to 'test/functional/fixtures/kbn_archiver/kibana_sample_data_flights_index_pattern'

2 issues with this test;

  1. the test initially got broken by the removed deprecated getHourOfDay painless function which was replaced by getHour(). I just changed the scripted field to use the new method in the new kbnArchive file
  2. when you apply a filter in the vis editor, it doesn't show the values but instead shows Warning. I changed the tests to accept that for now. The other option is to then add the controls to a dashboard, with another visualizations I think, and then it would actually show the values. I haven't done that yet. Need input from the Visualization team.

This was referenced Mar 8, 2022
Closed
Closed
Closed
Closed
@LeeDr
Copy link
Author

LeeDr commented Mar 14, 2022

@elasticmachine merge upstream

@LeeDr LeeDr added v8.2.0 v8.1.1 v7.17.2 Team:QA Team label for QA Team release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated - use backport:version if exact versions are needed test_ui_functional test_xpack_functional Feature:Functional Testing labels Mar 15, 2022
@LeeDr LeeDr marked this pull request as ready for review March 15, 2022 00:44
@LeeDr LeeDr requested a review from a team as a code owner March 15, 2022 00:44
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-qa (Team:QA)

ThomThomson
ThomThomson reviewed Mar 15, 2022
View reviewed changes
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this test @LeeDr!

So the warning is expected because the Input Controls don't communicate their data views back to the query bar. What is interesting to me, though, is that the test used to work with the actual values rather than warning. This is interesting because this behaviour hasn't changed at all since the Input Controls were created.

This makes me think that the flights demo data used to be the default data view, but now it is not. When the Input Control is created against the default data view, the pills it applies work correctly without the warning. Can you think of any reason why the flights data isn't default anymore? Should we change it so it is default for these tests?

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Test Failures

  • [job] [logs] Default CI Group #10 / Actions and Triggers app alerts list should be able to mute the rule with non "alerts" consumer from a non editable context

Metrics [docs]

✅ unchanged

History

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

ThomThomson
ThomThomson approved these changes Mar 15, 2022
View reviewed changes
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Changes LGTM! Code only review

@LeeDr LeeDr merged commit de29e5a into elastic:main Mar 16, 2022
@kibanamachine kibanamachine mentioned this pull request Mar 16, 2022
Closed
kibanamachine pushed a commit that referenced this pull request Mar 16, 2022
[Archive Migration] kibana_sample_data_flights_index_pattern (#127230)
b96b5c2 
* mostly fixed, but shows warning

* Fix test, but accept Warnings in filters

* remove superuser role from debugging

* Remove unneeded comment

* set default data view and expect actual results

Co-authored-by: Kibana Machine <[email protected]>
(cherry picked from commit de29e5a)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
✅ 8.1
❌ 7.17 Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 127230

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

maksimkovalev pushed a commit to maksimkovalev/kibana that referenced this pull request Mar 18, 2022
@kibanamachine @maksimkovalev
[Archive Migration] kibana_sample_data_flights_index_pattern (elastic…
de7edec 
…#127230)

* mostly fixed, but shows warning

* Fix test, but accept Warnings in filters

* remove superuser role from debugging

* Remove unneeded comment

* set default data view and expect actual results

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 18, 2022
@kibanamachine
Copy link
Contributor

Looks like this PR has a backport PR but it still hasn't been merged. Please merge it ASAP to keep the branches relatively in sync.

@LeeDr LeeDr mentioned this pull request Mar 18, 2022
Merged
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

4 similar comments
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@LeeDr LeeDr removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 25, 2022
@LeeDr LeeDr deleted the kbn-sampleFlights branch March 25, 2022 16:24
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 28, 2022
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@LeeDr LeeDr removed backport missing Added to PRs automatically when the are determined to be missing a backport. auto-backport Deprecated - use backport:version if exact versions are needed labels Mar 28, 2022
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 29, 2022
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

1 similar comment
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@LeeDr LeeDr removed backport missing Added to PRs automatically when the are determined to be missing a backport. v7.17.2 labels Mar 31, 2022
@LeeDr
Copy link
Author

LeeDr commented Mar 31, 2022

I removed the v7.17.2 label even though this WAS backported and merged into 7.17 branch because github won't stop complaining about backport missing?!?!

@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 31, 2022
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@LeeDr LeeDr added backport:skip This commit does not require backporting and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThomThomson ThomThomson ThomThomson approved these changes

Assignees
No one assigned
Labels
backport:skip This commit does not require backporting Feature:Functional Testing release_note:skip Skip the PR/issue when compiling release notes Team:QA Team label for QA Team test_ui_functional test_xpack_functional v8.1.1 v8.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@LeeDr @elasticmachine @kibana-ci @kibanamachine @ThomThomson