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

[Mappings editor] Add component integration tests #63853

Merged
merged 46 commits into from
May 7, 2020

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented Apr 17, 2020

This PR adds component integration tests coverage to the mappings editor.

What is currently tested

Core

  • Default behavior of the mappings editor: converting a field with no type defined to an object types and adding default _meta and _source objects.
  • Component props (value and onChange): make sure the component renders the correct values and that it forwards to the consumer component the changes.

Mapped fields

  • Make sure the correct DOM tree renders for the provided mappings properties
  • Make sure the mappings editor can be controlled and that the DOM tree updates when a new value is provided

Edit field

  • Make sure the correct field is edited (title & path)
  • Advanced settings are hidden by default
  • Changing field datatype updates the form correctly with the new parameters

Datatypes

Text

  • Make sure the correct default parameters are sent back.
  • Default values for searchable, index analyzer, search analyzer and search_quote analyzer
  • When analyzer with sub-select (e.g. language -> french), make sure both dropdown selects have the correct default value
  • Checkbox for the "Use same analyzer for search" logic
  • Allow custom analyzer to be inserted manually with a text input
  • Provide Index settings to the Mappings editor and verify that the custom analyzers are displayed under 2 dropdown selects "Custom -> "

Shape

  • Make sure the correct default parameters are sent back.

Bug fixes

The following bugs have been fixed

  • In the "Advanced options", the "Map numeric strings as numbers" toggle was controlling the value of the "Throw an exception when a document contains an unmapped field" (visible when "Enable dynamic mappings" is set to false). Updating one was updating the other.
  • In the text analyzers, when there is a second "sub" select (ex. for "languages"), the default value was wrongly set on the first option. So if we were loading a "french" analyzer, it would display "Arabic" in the dropdown.

Other changes

I also updated the Mappings editor props to follow the convention

  • value instead of defaultValue as the component can be controlled by the consumer and updates its internal state accordingly
  • onChange instead of `onUpdate

cc @cuff-links

@sebelga sebelga added Feature:Mappings Editor Index mappings editor UI Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more test-jest-integration v8.0.0 labels Apr 22, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/es-ui (Team:Elasticsearch UI)

@sebelga sebelga added the v7.8.0 label Apr 22, 2020
@sebelga sebelga force-pushed the test/mappings-editor branch 2 times, most recently from d85f23c to fb4d980 Compare April 27, 2020 09:26
@sebelga
Copy link
Contributor Author

sebelga commented May 2, 2020

@elasticmachine merge upstream

1 similar comment
@cuff-links
Copy link
Contributor

@elasticmachine merge upstream

@sebelga
Copy link
Contributor Author

sebelga commented May 4, 2020

@cuff-links Thanks for the review! To modify the jest script this would be something to ask to the operation team if we think it is worth it.
What I think would be more interesting is to be able to run the Jest test 50 times for a specific plugin in isolation.

@sebelga
Copy link
Contributor Author

sebelga commented May 4, 2020

@elasticmachine merge upstream

@sebelga
Copy link
Contributor Author

sebelga commented May 4, 2020

@elasticmachine merge upstream

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Great work, Seb! Code looks great overall. I had a few suggestions that would have helped me understand the tests and also a couple of questions. I'd love to see them addressed but I don't think we need to block on them.

@sebelga
Copy link
Contributor Author

sebelga commented May 6, 2020

@elasticmachine merge upstream

@sebelga
Copy link
Contributor Author

sebelga commented May 6, 2020

@elasticmachine merge upstream

@sebelga
Copy link
Contributor Author

sebelga commented May 6, 2020

Thanks for the review @cjcenizal ! I address most of your concerns 👍

@sebelga
Copy link
Contributor Author

sebelga commented May 7, 2020

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@sebelga sebelga merged commit 83a088c into elastic:master May 7, 2020
@sebelga sebelga deleted the test/mappings-editor branch May 7, 2020 09:47
@sebelga sebelga added v7.9.0 and removed v7.8.0 labels May 7, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 7, 2020
…ttional-flaky

* upstream/master: (49 commits)
  Move remaining home assets to the new platform (elastic#65053)
  Change the copy and the id from blacklist to block list for consistency (elastic#65419)
  [ML] Hide selector helper in Anomaly Explorer swimlane (elastic#65522)
  [ML] Fix the limit control on the Anomaly explorer page (elastic#65459)
  [Mappings editor] Add component integration tests (elastic#63853)
  [Logs + Metrics UI] Prevent component errors from breaking the whole UI (elastic#65456)
  [Logs UI] Disable search bar when live stream is on. (elastic#65491)
  fix SavedObjectMigrationMap type (elastic#65569)
  [Uptime] Improve cert flaky test (elastic#65458)
  [Uptime] Fix monitor list result runtime type, ip can be null (elastic#65532)
  [APM] Agent configuration: Bug makes it possible to create invalid configurations (elastic#65508)
  [APM] Remove link from active page in the breadcrumb (elastic#65473)
  [SIEM] Fixes test flakiness (elastic#65510)
  [ESLint] update @kbn/eslint/no-restricted-paths rule to allow imports mocks from folder (elastic#65471)
  Migrate test plugins ⇒ NP (kbn_tp_run_pipeline) (elastic#64780)
  move core provier to NP. allows to run tests on every page (elastic#64929)
  Extended alerting documentation with information about using Kibana keystore and action types for preconfigured connectors (elastic#65201)
  [functional tests] add some missing awaits (elastic#65566)
  Fixed create new connector from alert flyout form throw an error messages in external plugins. (elastic#65539)
  [SIEM] [Cases] External services not getting all comments bug fix (elastic#65307)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 7, 2020
* master:
  [ML] Transforms: Fix API error message display for edit flyout. (elastic#65494)
  [SIEM][Detection Engine] Fixes import bug with non existent signals index (elastic#65595)
  [Lens] Use rules of hooks with linting (elastic#65593)
  [ML] Migrate server side Mocha tests to Jest. (elastic#65651)
  Fixes the client to setup SSL with the CA certificates for testing (elastic#65598)
  reduce uptime plugin initial bundle size (elastic#65257)
  [ML] Consolidating shared types and util functions (elastic#65247)
  [Drilldowns] Preserve state when selecting different action factory (elastic#65074)
  Migrate test plugins ⇒ NP (kbn_tp_embeddable_explorer) (elastic#64756)
  Move remaining home assets to the new platform (elastic#65053)
  Change the copy and the id from blacklist to block list for consistency (elastic#65419)
  [ML] Hide selector helper in Anomaly Explorer swimlane (elastic#65522)
  [ML] Fix the limit control on the Anomaly explorer page (elastic#65459)
  [Mappings editor] Add component integration tests (elastic#63853)
  [Logs + Metrics UI] Prevent component errors from breaking the whole UI (elastic#65456)
  [Logs UI] Disable search bar when live stream is on. (elastic#65491)
gmmorris added a commit to gmmorris/kibana that referenced this pull request May 7, 2020
…ponents

* alerting/lazy-load-actions:
  align and style loading indicator
  [ML] Transforms: Fix API error message display for edit flyout. (elastic#65494)
  [SIEM][Detection Engine] Fixes import bug with non existent signals index (elastic#65595)
  [Lens] Use rules of hooks with linting (elastic#65593)
  [ML] Migrate server side Mocha tests to Jest. (elastic#65651)
  Fixes the client to setup SSL with the CA certificates for testing (elastic#65598)
  reduce uptime plugin initial bundle size (elastic#65257)
  [ML] Consolidating shared types and util functions (elastic#65247)
  [Drilldowns] Preserve state when selecting different action factory (elastic#65074)
  Migrate test plugins ⇒ NP (kbn_tp_embeddable_explorer) (elastic#64756)
  Move remaining home assets to the new platform (elastic#65053)
  Change the copy and the id from blacklist to block list for consistency (elastic#65419)
  [ML] Hide selector helper in Anomaly Explorer swimlane (elastic#65522)
  [ML] Fix the limit control on the Anomaly explorer page (elastic#65459)
  [Mappings editor] Add component integration tests (elastic#63853)
  [Logs + Metrics UI] Prevent component errors from breaking the whole UI (elastic#65456)
  [Logs UI] Disable search bar when live stream is on. (elastic#65491)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Mappings Editor Index mappings editor UI non-issue Indicates to automation that a pull request should not appear in the release notes release_note:skip Skip the PR/issue when compiling release notes Team:Kibana Management Dev Tools, Index Management, Upgrade Assistant, ILM, Ingest Node Pipelines, and more test-jest-integration v7.9.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants