[Enterprise Search] Mocks/tests tech debt - avoid hungry mocking by cee-chen · Pull Request #101107 · elastic/kibana

cee-chen · 2021-06-01T21:03:18Z

Summary

Don't be scared by the 270 changed files! The changes to look at are the ones in public/applications/__mocks__ - everything else is only an import update to account for the new mocks architecture/folder structure.

Here's a quick screenshot of the changes:

Changes:

test_helpers (eaca116)
- These were files that didn't really belong in __mocks__ because they weren't mocks. They were test helpers, something Kibana provides for in its dev directories checks.
react_router (ebb1d7a)
- This move was the original inspiration for this PR. Scotty initiated a Slack convo a few weeks/months ago voicing confusion at the fact React Router was silently getting mocked from him doing a basic import { someKeaMock } from '../../__mocks__';. Why would a Kea mock cause RR to get mocked? An excellent question, and it was because we were mocking/exporting everything wholesale out of __mocks__/index.ts, which this PR now removes the ambiguity of by only grouping certain logical exports together (e.g., Kea).
- For some context, this "hungry mock" behavior was originally introduced in [Enterprise Search] Automatically mock shared logic files #88494 for convenience. However, it's become apparently that explicitness trumps convenience/shortcuts here in terms of developer understanding/experience. 🏆
kea_logic (ed7743b)
- Just for further organization/tidiness, I moved all our various Kea logic mocks & test helpers (e.g. useActions/useValues mocks, LogicMounter) into its own folder as well.
- And to be even more extra, I split up the LogicMounter class into its own file separate from the previous kea.mock.ts (which I renamed to hooks.mock.ts, since it's essentially in charge of just the useActions/useValues mocks).
- Unfortunately I hilariously found out that naming said folder __mocks__/kea/ led to Jest auto-mocking all tests importing Kea, which we don't want, so I had to slightly more verbosely name the folder kea_logic 🤦‍♀️ Hope that's cool with y'all.

Checklist

This is a tech debt / test only PR. All unit tests should pass, nothing else should change at all

Unit or functional tests were updated or added to match the most common scenarios

…elpers/ They're not technically mocks since nothing is being mocked, so we should move them into a test_helpers folder for specificity & organization

This was part of the initial feedback, that it was unclear why importing something for Kea in __mocks__/index.ts was mocking react router along for the ride. Separating this out makes things clearer and imports more explicit + add some handy new mock useX jest.fn()s helpers, so we're not doing `useParams() as jest.Mock` errywhere

- for organization NOTE: It can't be a plain kea/ folder because then Jest automatically mocks the `kea` module itself kea 🤦

cee-chen · 2021-06-01T21:06:57Z

Also want to add no rush on this PR at all - it's meant to be a minor tech debt / dev QOL enhancement only, so if you disagree that it's actually a dev experience improvement or just want to address features or bugs first, feel free to let this sit & percolate a bit.

(It'll also cause a merge conflict with Jason's currently open Kea test PR (#100134) because I pulled out LogicMounter into its own file, so apologies for that and I'm happy to wait to merge after that lands 🙏)

JasonStoltz

LGTM once this goes green.

x-pack/plugins/enterprise_search/public/applications/__mocks__/react_router/index.ts

- Caused by switch from any to unknown (changed back to any + added a .test_helper suffix exclusion for any)

- I checked all application folders but this one, whoops

- null not being type-able as a boolean - forgot to remove various useParam imports after adding mockUseParams + misc unused kea import, probably added while debugging kea mocks

scottybollinger

This is great Constance. Really grateful for all of these helpers and for taking the time to make them better! 🎉

kibanamachine · 2021-06-08T19:35:56Z

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

💔 Build #128503 failed df7177b

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

kibanamachine · 2021-06-09T04:38:51Z

💔 Backport failed

Status	Branch	Result
❌	7.x	Commit could not be cherrypicked due to conflicts

To backport manually run:
node scripts/backport --pr 101107

* master: (54 commits) Implement "select all" rules feature (elastic#100554) [ML] Remove script fields from the Anomaly detection alerting rule executor (elastic#101607) [Security solutions][Endpoint] Update event filtering texts (elastic#101563) [Enterprise Search] Mocks/tests tech debt - avoid hungry mocking (elastic#101107) [FTR] Updates esArchive paths [FTR] Updates esArchive paths [Security Solution][Detection Engine] Adds runtime field tests (elastic#101664) Added APM PHP agent to the list of agent names (elastic#101062) [CI] Restore old version_info behavior when .git directory is present (elastic#101642) [Fleet] Add fleet server telemetry (elastic#101400) [APM] Syncs agent config settings to APM Fleet policies (elastic#100744) [esArchiver] drop support for --dir, use repo-relative paths instead (elastic#101345) Revert "[xpack/test] restore incremental: false in ts project" [Security Solution] Remove Host Isolation feature flag (elastic#101655) [xpack/test] restore incremental: false in ts project [DOCS] Adds link to video landing page (elastic#101413) [ML] Move Index Data Visualizer into separate plugin (Part 1) (elastic#100922) Improve security plugin return types (elastic#101492) [ts] migrate `x-pack/test` to composite ts project (elastic#101441) [App Search] Updated Search UI to new URL (elastic#101320) ...

…1107) (#101788) * Move enzyme & misc test helpers out of __mocks__/ and into new test_helpers/ They're not technically mocks since nothing is being mocked, so we should move them into a test_helpers folder for specificity & organization * Move React Router mocks into its own separate folder/import This was part of the initial feedback, that it was unclear why importing something for Kea in __mocks__/index.ts was mocking react router along for the ride. Separating this out makes things clearer and imports more explicit + add some handy new mock useX jest.fn()s helpers, so we're not doing `useParams() as jest.Mock` errywhere * Move Kea & logic mocks/helpers into kea_logic subfolder - for organization NOTE: It can't be a plain kea/ folder because then Jest automatically mocks the `kea` module itself kea 🤦 * Fix type failures - Caused by switch from any to unknown (changed back to any + added a .test_helper suffix exclusion for any) * Fix Enterprise Search tests/imports - I checked all application folders but this one, whoops * PR feedback: comment copy * Update tests/files added since PR open with new import locations * Fix misc react router typing - null not being type-able as a boolean - forgot to remove various useParam imports after adding mockUseParams + misc unused kea import, probably added while debugging kea mocks # Conflicts: # x-pack/plugins/enterprise_search/public/applications/app_search/components/crawler/crawler_overview.test.tsx # x-pack/plugins/enterprise_search/public/applications/app_search/components/crawler/crawler_overview_logic.test.ts

cee-chen added 3 commits June 1, 2021 13:22

Move enzyme & misc test helpers out of __mocks__/ and into new test_h…

eaca116

…elpers/ They're not technically mocks since nothing is being mocked, so we should move them into a test_helpers folder for specificity & organization

Move Kea & logic mocks/helpers into kea_logic subfolder

df7177b

- for organization NOTE: It can't be a plain kea/ folder because then Jest automatically mocks the `kea` module itself kea 🤦

cee-chen added technical debt Improvement of the software architecture and operational architecture release_note:skip Skip the PR/issue when compiling release notes v7.14.0 auto-backport Deprecated - use backport:version if exact versions are needed labels Jun 1, 2021

cee-chen requested review from a team June 1, 2021 21:03

JasonStoltz approved these changes Jun 2, 2021

View reviewed changes

x-pack/plugins/enterprise_search/public/applications/__mocks__/react_router/index.ts Outdated Show resolved Hide resolved

cee-chen added 6 commits June 8, 2021 08:21

Merge branch 'master' into mocks-tech-debt

d5ccb7c

Fix type failures

cb73f09

- Caused by switch from any to unknown (changed back to any + added a .test_helper suffix exclusion for any)

Fix Enterprise Search tests/imports

ebe28ab

- I checked all application folders but this one, whoops

PR feedback: comment copy

0d94ee1

Update tests/files added since PR open with new import locations

7c4bb4e

Fix misc react router typing

338c687

- null not being type-able as a boolean - forgot to remove various useParam imports after adding mockUseParams + misc unused kea import, probably added while debugging kea mocks

scottybollinger approved these changes Jun 8, 2021

View reviewed changes

cee-chen merged commit 12986fb into elastic:master Jun 9, 2021

cee-chen deleted the mocks-tech-debt branch June 9, 2021 04:38

cee-chen mentioned this pull request Jun 9, 2021

[7.x] [Enterprise Search] Mocks/tests tech debt - avoid hungry mocking (#101107) #101788

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Enterprise Search] Mocks/tests tech debt - avoid hungry mocking#101107

[Enterprise Search] Mocks/tests tech debt - avoid hungry mocking#101107
cee-chen merged 9 commits intoelastic:masterfrom
cee-chen:mocks-tech-debt

cee-chen commented Jun 1, 2021 •

edited

Loading

Uh oh!

cee-chen commented Jun 1, 2021

Uh oh!

JasonStoltz left a comment

Uh oh!

Uh oh!

scottybollinger left a comment

Uh oh!

kibanamachine commented Jun 8, 2021

Uh oh!

kibanamachine commented Jun 9, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

cee-chen commented Jun 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

cee-chen commented Jun 1, 2021

Uh oh!

JasonStoltz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

scottybollinger left a comment

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Jun 8, 2021

💚 Build Succeeded

Metrics [docs]

History

Uh oh!

kibanamachine commented Jun 9, 2021

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

cee-chen commented Jun 1, 2021 •

edited

Loading