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

[Enterprise Search] Allow jest script to run on individual files #96589

Merged
merged 4 commits into from
Apr 12, 2021

Conversation

JasonStoltz
Copy link
Member

@JasonStoltz JasonStoltz commented Apr 8, 2021

Summary

This allows the jest test script for the enterprise_search plugin to run on files, as well as directories.

Example:

sh jest.sh public/applications/app_search/components/result_settings/result_settings_logic.test.ts 

I tested with file names (both test.tsx, test.ts), and folder with and without a trailing /. Everything seems to work as usual.

For maintainers

This allows the jest test script for the enterprise_search plugin
to run on files, and well as directories.

Previously, a trailing slash was always appended, whether it was
a file name, or a directory. It now maintains whatever a user originally
inputted.
@JasonStoltz JasonStoltz requested a review from a team April 8, 2021 15:33
@JasonStoltz JasonStoltz added v7.13.0 auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes and removed auto-backport Deprecated - use backport:version if exact versions are needed labels Apr 8, 2021
@cee-chen
Copy link
Contributor

cee-chen commented Apr 8, 2021

BTW, it does looks like 7.x has the jest.sh script so we should probably backport this still? 🤷

@JasonStoltz JasonStoltz added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 8, 2021
Copy link
Contributor

@byronhulcher byronhulcher 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 doing this! Looks good to me but I'm deferring to Constance since she left some comments

@JasonStoltz JasonStoltz requested a review from cee-chen April 8, 2021 15:52
@cee-chen
Copy link
Contributor

cee-chen commented Apr 8, 2021

[optional / I can do this in a separate PR if you prefer] I totally brainfarted when testing this and tripped up on whether I should be testing a .test file or the source file. Is there any chance we can update our README.md (https://github.com/elastic/kibana/blob/master/x-pack/plugins/enterprise_search/README.md) under the unit tests section to show an example or two for specific file testing, and include a quick comment that we should be targeting .test files? Thank you again!! This is terrific!

@JasonStoltz
Copy link
Member Author

JasonStoltz commented Apr 12, 2021

@constancecchen Good call! I didn't realize we had a README about that. I have to update from master anyway so I'll make that change. a3a0142

@JasonStoltz JasonStoltz enabled auto-merge (squash) April 12, 2021 15:18
@cee-chen
Copy link
Contributor

Looks terrific - thanks a million! 🙇‍♀️

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 12, 2021
@JasonStoltz JasonStoltz deleted the jest-script-fix branch April 22, 2021 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated - use backport:version if exact versions are needed release_note:skip Skip the PR/issue when compiling release notes v7.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants