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

[test_utils/Testbed] Move to src/test_utils folder (OSS) #66898

Merged
merged 26 commits into from
May 25, 2020
Merged

[test_utils/Testbed] Move to src/test_utils folder (OSS) #66898

merged 26 commits into from
May 25, 2020

Conversation

sebelga
Copy link
Contributor

@sebelga sebelga commented May 18, 2020 •
edited
Loading

This PR moves the "test_utils/testbed" folder and its dependencies ("lib" and "helpers") to the OSS "src/es_ui_shared" folder.

This is required so we can test code in our "es_ui_shared" folder, like the form library.

In this PR I also deleted the temporary copy of the test_utils that I had made in #64647.

Note to reviewer

As the only changes are related to test helpers imports, if the CI is green everything should be fine. Maybe make sure I haven't accidentally modified a non-test file.

[EDIT] The intention of this PR is not doable with the current build process. It seems to be because putting test-related code (that depends on jest to be present globally. e.g. jest.Mock, jest.fn()) does not work inside a plugin as they are built differently. I decided then to move the copy of the testbed and the utils it depends on, out of the "es_ui_shared" plugin and into "src/test_utils" folder. It seems that we already maintain 2 copies of enzyme_helpers.ts for that same reason.
If we find a solution for the build to pass, then we can revert my 3 last commits (a1d94f8, 516ecb7, 6e6807f)
cc @jloleysens @mistic

@sebelga sebelga requested a review from jloleysens May 18, 2020 14:58
@sebelga sebelga marked this pull request as ready for review May 18, 2020 14:59
@sebelga sebelga requested a review from a team as a code owner May 18, 2020 14:59
@sebelga sebelga mentioned this pull request May 18, 2020
Open
24 tasks
@sebelga sebelga added 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 v7.9.0 v8.0.0 labels May 18, 2020
@elasticmachine
Copy link
Contributor

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

@sebelga sebelga force-pushed the testbed/move-to-oss branch from 1fcb62a to de4470b Compare May 18, 2020 16:00
jloleysens
jloleysens approved these changes May 19, 2020
View reviewed changes
Copy link
Contributor

@jloleysens jloleysens left a comment

Choose a reason for hiding this comment

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

Happy for this to be merged when CI is green. Great work @sebelga !

Most CI failures seem to be related to src/plugins/es_ui_shared not existing anymore. Happy to see our object based namespacing be used too!

@sebelga
Copy link
Contributor Author

sebelga commented May 20, 2020

Thanks for the review @jloleysens !

Most CI failures seem to be related to src/plugins/es_ui_shared not existing anymore.

Strangely, only for searchprofiler it does not resolve the absolute path to "src/plugins" (see logs https://kibana-ci.elastic.co/job/elastic+kibana+pipeline-pull-request/48968/execution/node/259/log/)

I had the same issue for .js files but all .ts file resolved correctly with absolute paths. So for searchprofiler I had to put back the relative paths 😞 (2609520)

@tylersmalley tylersmalley mentioned this pull request May 20, 2020
Merged
@mistic
Copy link
Member

mistic commented May 20, 2020

@elasticmachine merge upstream

@sebelga
Copy link
Contributor Author

sebelga commented May 20, 2020

@elasticmachine merge upstream

@mistic mistic mentioned this pull request May 22, 2020
Closed
@sebelga
Copy link
Contributor Author

sebelga commented May 24, 2020

@elasticmachine merge upstream

@sebelga
Copy link
Contributor Author

sebelga commented May 25, 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 changed the title [test_utils/Testbed] Move to OSS [test_utils/Testbed] Move to src/test_utils folder (OSS) May 25, 2020
@sebelga sebelga merged commit 03b6c6d into elastic:master May 25, 2020
@sebelga sebelga deleted the testbed/move-to-oss branch May 25, 2020 09:38
@sebelga sebelga restored the testbed/move-to-oss branch May 25, 2020 09:38
@sebelga sebelga mentioned this pull request May 25, 2020
Merged
sebelga added a commit that referenced this pull request May 25, 2020
@sebelga
[7.x] [test_utils/Testbed] Move to src/test_utils folder (OSS) (#66898)…
33a8160 
… (#67261)
jloleysens added a commit that referenced this pull request May 26, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into feature/pipel…
f06f9e4 
…ine-editor

* 'master' of github.com:elastic/kibana: (129 commits)
  [Canvas] Force embeddables to refresh when renderable reevaluated (#67133)
  [Canvas] Better handling navigating to/from canvas (#66407)
  [Ingest pipelines] Fix schema validation for simulate and update routes (#67199)
  do not use es from setup (#67277)
  Auto expand replicas for event log (#67286)
  Observability & APM do not use elasticsearch client provided via setup contract  (#67263)
  Fix privileges check when security is not enabled (#67308)
  add IIS home (#66918)
  [ML] Adding additional job service endpoint tests (#66892)
  [Ingest Manager] Update fleet internal doc with latest flags (#67193)
  [Discover] Deangularize the loading spinner (#67165)
  Add `application.navigateToUrl` core API (#67110)
  Improve indexpattern without timefield functional test (#67031)
  KibanaContext in index pattern managment ui (#66985)
  Fix Azure metrics tutorial inside the App Home/ Add data area (#66901)
  add azure logs home (#66910)
  fix: rum agent should work correctly on new platform (#67037)
  [test_utils/Testbed] Move to src/test_utils folder (OSS) (#66898)
  only block registration when appRoute contains the exact basePath (#67125)
  Changed actions API endpoints urls to follow Kibana STYLEGUIDE (#65936)
  ...

# Conflicts:
#	x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_fields.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jloleysens jloleysens jloleysens approved these changes

Assignees
No one assigned
Labels
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 v7.9.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@sebelga @elasticmachine @mistic @kibanamachine @jloleysens