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] Add Kea test helper for directly accessing listeners #89061

Merged
merged 3 commits into from
Jan 25, 2021

Conversation

cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Jan 22, 2021

Summary

Jason is a Kea guru by now and has started using breakpoints in listeners to help debounce calls.

Unfortunately, this adds some amount of async shenanigans to Jest unit tests when calling the listener via SomeLogic.actions.someListener.

Accessing the listener fn directly (rather than the wrapped action) makes async testing about easier or possible to catch errors/await breakpoints in the first place. Thus: this getListeners helper 👂

The actual implementation isn't too difficult (I originally just console logged the logic file until I got what I wanted, which was the listener fns), but pulling it out to a helper makes it little more reusable & readable.

Example usage:

const { mount, getListeners } = new LogicMounter(SomeLogic);

it('some test', async () => {
  mount();

  // If your specific listener doesn't depend on any values or actions, just do `getListeners()` w/o args
  const { someListener } = getListeners({
    values: { someMockValue: false },
    actions: { someMockAction: jest.fn() },
  });

  const mockBreakpoint = jest.fn();
  await someListener({ someMockArgument: true }, mockBreakpoint);
}); 

Checklist

@cee-chen cee-chen added Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes labels Jan 22, 2021
@cee-chen cee-chen requested review from JasonStoltz, scottybollinger and a team January 22, 2021 17:32
@cee-chen cee-chen force-pushed the kea-get-listeners-helper branch from 6d7a19a to 0827738 Compare January 22, 2021 17:37
@cee-chen cee-chen force-pushed the kea-get-listeners-helper branch from 0827738 to fadebf9 Compare January 22, 2021 17:42
@cee-chen cee-chen force-pushed the kea-get-listeners-helper branch from fadebf9 to f18af00 Compare January 22, 2021 17:43
@cee-chen cee-chen removed the request for review from scottybollinger January 22, 2021 17:43
@cee-chen
Copy link
Contributor Author

@scottybollinger Sorry for the accidental assignment/ping, I originally shoved some extra organizational mocking stuff in this PR that touched WS files, but then realized I didn't need to / could work around it.

Just FYI you don't have to import LogicMounter directly from __mocks__/kea.mock btw, you can grab it straight from __mocks__ itself :)

@scottybollinger scottybollinger removed the request for review from a team January 22, 2021 19:47
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.

This seems like a better approach than getting these listeners ad-hoc in each logic file. Is it worth creating an issue in the main kea repo about this? This doesn't seem intuitive, and testing side effects for listeners is a valid use case, I'm surprised this is "the way" to do it.

@JasonStoltz
Copy link
Member

@elasticmachine merge upstream

@JasonStoltz
Copy link
Member

Testing listeners has been the source of more than a few lost development hours. This simplifies them greatly by simply allowing us to test them as plain old functions. This should let us crank out tests much quicker.

It's a slightly less realistic test overall because we're not invoking the listener as it would actually be invoked using Kea.

However, I think it's a reasonable tradeoff.

Thanks for doing this Constance!

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

@cee-chen
Copy link
Contributor Author

@scottybollinger going to merge this PR in as-is, I'm thinking I'll use a separate helper for the things we chatted about today (mock breakpoint, invoke action for test coverage)

@cee-chen cee-chen merged commit 474af9f into elastic:master Jan 25, 2021
@cee-chen cee-chen deleted the kea-get-listeners-helper branch January 25, 2021 20:15
cee-chen pushed a commit that referenced this pull request Jan 25, 2021
…ers (#89061) (#89223)

* Add getListeners to Kea test helpers

* Update TelemetryLogic to use new getListeners helper

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Plugins release_note:skip Skip the PR/issue when compiling release notes v7.12.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants