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

Failing test: Chrome UI Functional Tests.test/functional/apps/dashboard_elements/controls/options_list·ts - dashboard elements dashboard elements Controls Dashboard options list integration Interactions between options list and dashboard test data view runtime field making selection has expected results #146086

Closed
kibanamachine opened this issue Nov 23, 2022 · 10 comments · Fixed by #144867
Labels
failed-test A test failure on a tracked branch, potentially flaky-test Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas

Comments

@kibanamachine
Copy link
Contributor

kibanamachine commented Nov 23, 2022

A test failed on a tracked branch

Error: expected [ 'hiss', 'ruff', 'bark', 'grrr', 'meow' ] to sort of equal [ 'bark', 'bow ow ow' ]
    at Assertion.assert (node_modules/@kbn/expect/expect.js:100:11)
    at Assertion.eql (node_modules/@kbn/expect/expect.js:244:8)
    at Context.<anonymous> (test/functional/apps/dashboard_elements/controls/options_list.ts:430:57)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
    at Object.apply (node_modules/@kbn/test/target_node/src/functional_test_runner/lib/mocha/wrap_function.js:78:16) {
  actual: '[\n  "hiss"\n  "ruff"\n  "bark"\n  "grrr"\n  "meow"\n]',
  expected: '[\n  "bark"\n  "bow ow ow"\n]',
  showDiff: true
}

First failure: CI Build - main

@kibanamachine kibanamachine added the failed-test A test failure on a tracked branch, potentially flaky-test label Nov 23, 2022
@botelastic botelastic bot added the needs-team Issues missing a team label label Nov 23, 2022
@kibanamachine kibanamachine added the Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas label Nov 23, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

@botelastic botelastic bot removed the needs-team Issues missing a team label label Nov 23, 2022
@kibanamachine
Copy link
Contributor Author

New failure: CI Build - main

@mistic
Copy link
Member

mistic commented Nov 24, 2022

Skipped.

main: b9280d1

@ThomThomson
Copy link
Contributor

@Heenawter, would you mind taking a look at why this one became flaky suddenly?

@Heenawter
Copy link
Contributor

@ThomThomson Looking into it already 👍

@Heenawter
Copy link
Contributor

Oh! Looks like I actually caught this flakiness and attempted to fix it in #144867 (comment addressing this is here).

So once that PR goes in, I think the flakiness should be fixed 👀

Heenawter added a commit that referenced this issue Nov 24, 2022
…144867)

Closes #140174
Closes #145040
Closes #146086 

## Summary

This PR adds two features to the options list control:
1. A button in the options list popover that gives users the ability to
change how the suggestions are sorted
    <p align="center">
<img
src="https://user-images.githubusercontent.com/8698078/203416853-58f9c909-8909-4902-adf3-59831018c96f.gif"/>
    </p>

2. A per-control setting that disables the ability to dynamically sort
which, if set to `false`, presents the author with the ability to select
one of the four sorting methods for that specific control to use
    <p align="center">
<img
src="https://user-images.githubusercontent.com/8698078/203417193-cd35f264-8c29-4c80-b88b-15da25a1f56c.gif"/>
    </p>

### Design considerations
@elastic/kibana-design 

As noted by Andrea when looking at the preliminary behaviour of this
feature, the `"Show only selected"` toggle has increased in importance
because of the new sorting mechanic - after all, when making selections
and then changing the sort method, your selections can appear to be
"lost" if you have enough unique values in the control's field.

In the original designs, the `"Clear all selections"` button was
**first** in the popover's action bar - however, I found that I kept
accidentally clicking this in my testing when switching between
searching, sorting, making selections, changing sorting, showing only
selected options, etc. etc. I found that the following design felt a lot
more natural for the placement of the `"Clear all selections"` button:


![image](https://user-images.githubusercontent.com/8698078/202318768-cf8a5668-40c4-482f-9eb0-023508866068.png)

Note that, once #143585 is
resolved, this will no longer be as much of a concern because we will be
moving, at the very least, the `"Clear all selections"` to be a floating
action. That being said, this new order for the actions is, in my
opinion, a good compromise in the mean time. Very much open to feedback,
though!

### Video 


https://user-images.githubusercontent.com/8698078/203422674-52aac87c-7295-4eb6-99a5-ee3ffba2756b.mov


### Testing Notes
There are a few things to consider when testing:
1. Does the dynamic sorting give you expected results when sorting
various field types?
- Note that IP fields only support document count sorting, so ensure
that "Alphabetical" sorting does not show up in the sorting list during
either creation or as part of the popover sorting.
2. When setting the `"Allow suggestions to be sorted"` toggle to
`false`, it should always default to `"Document count (descending)"` to
prevent invalid sort selections. For example, consider the following:
    - Create an options list control on some keyword field
- Set the sort to alphabetical (either ascending or descending) in the
popover
    - Edit that control and change it to an IP field
    - Set `"Allow suggestions to be sorted"` to `false
- The default sort should be `"Document count (descending)"` and **not**
`"Alphabetical (descending/ascending)"`, since alphabetical sorting
would be invalid in this case.

**Flaky Test Runner**
<a
href="https://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/1585"><img
src="https://user-images.githubusercontent.com/8698078/203428246-13f5ff9a-df0c-4cd5-a4ee-cf7a98792362.png"/></a>



### Checklist

Delete any items that are not applicable to this PR.

- [x] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [x] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
<p><img
src="https://user-images.githubusercontent.com/8698078/202545715-96daa0ab-8900-45cb-979f-20a83e622597.png"/></p>
- [x] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
jloleysens added a commit to jloleysens/kibana that referenced this issue Nov 28, 2022
* main: (30 commits)
  [Cloud Posture] test latest findings table sort (elastic#144668)
  [api-docs] 2022-11-28 Daily api_docs build (elastic#146359)
  [api-docs] 2022-11-27 Daily api_docs build (elastic#146353)
  [api-docs] 2022-11-26 Daily api_docs build (elastic#146350)
  [DataViews] Fix form validation UX when the same data view name already exists (elastic#146126)
  [Discover] Prevent agg based visualizations of Discover saved objects with adhoc data views (elastic#145583)
  [Health Gateway] Update response aggregation (elastic#145761)
  [api-docs] 2022-11-25 Daily api_docs build (elastic#146341)
  [Metric threshold rule] Adds new context variable for group by keys (elastic#145654)
  [Controls] [Portable Dashboards] Add control group renderer example plugin (elastic#146189)
  Refactor Observability Overview Page (elastic#146182)
  Send complete test data to xMatters, so it can create an alert (elastic#145431)
  [Dashboard] [Controls] Allow options list suggestions to be sorted (elastic#144867)
  Add open API specification for list connector types (elastic#145951)
  skip flaky suite (elastic#146086)
  [ML] Removing duplicate tooltip text (elastic#146308)
  Refactor Rules Page (elastic#146193)
  [DOCS] Alert limit for cases (elastic#145950)
  Extend session index fields mapping with a session creation timestamp. (elastic#145997)
  [Files] Move <Image /> component to `@kbn/shared-ux` package (elastic#145995)
  ...
@jbudz jbudz reopened this Dec 20, 2022
@jbudz
Copy link
Member

jbudz commented Dec 20, 2022

New failure: CI Build - 8.6

@kibanamachine
Copy link
Contributor Author

New failure: CI Build - 8.6

@Heenawter
Copy link
Contributor

Heenawter commented Jan 10, 2023

From what I can tell, the above fix was not backported to 8.6 because it was included as part of a major feature - sorry about that! Can draft a quick PR for only this flaky test fix, and backport it to hopefully prevent future failures 👍

@Heenawter
Copy link
Contributor

Closing for now - #148662 was merged to the 8.6 branch, which should fix this problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
failed-test A test failure on a tracked branch, potentially flaky-test Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants