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

feat: User Event core code #1405

Merged
merged 9 commits into from
May 10, 2023
Merged

feat: User Event core code #1405

merged 9 commits into from
May 10, 2023

Conversation

mdjastrzebski
Copy link
Member

@mdjastrzebski mdjastrzebski commented May 6, 2023

Summary

This PR contain core User Event code to be used by both type() and press() functions. The API is based on latest User Event v14 API.

This PR contains stub implementations for type() and press(), as well as some re-usable utils:

  • wait() based on User Event's implementation
  • createEventLogger testing util for inspecting the sequence of events.

Some of this stuff is quite opinionated, so if you have ideas of alternative approaches, e.g. to testing, pls voice them so that we have a commonly agreed base for User Event code.

Test plan

Some new tests added, before extracting from #1396 had 100% code coverage.

return;
}

return Promise.all([
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is very confusing to me, I don't understand really why the advanceTimers function is used only for fake timers. Here we wait for two promises, one being useful in case fake timers are used and the other in case real timers are used, is this because we don't want to use jestFakeTimersAreEnabled ?

Also I don't really see the point of having advanceTimers as an option, what could it be used for? We don't really support any other test runner aside from jest currently right? I don't really know what users are using as a test runner but I have the impression that in RN with all the required presets Jest is currently the only solution that's supported by most major libraries

Copy link
Member Author

Choose a reason for hiding this comment

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

This implementation is taken straight from actual User Event code: https://github.com/testing-library/user-event/blob/main/src/utils/misc/wait.ts

The way it works is that the first part: new Promise<void>(resolve => globalThis.setTimeout(() => resolve(), delay)), works for both real and fake timers.

The second part config.advanceTimers(delay) is configurable through userEvent.setup() call. By default UserEvent has () => Promise.resolve() there, which allows this function to work with real timers. Having it as an option allows to support different fake timer types, e.g. Sinon.JS and not just Jest ones.

My innovation in that regard is that I've created a universal advanceTimers default function that works with both real timers and fake Jest timers, which I assume are used by majority of our users using fake timers at all.

Having that as an option allows other fake timers users, still to use them with our User Event. And there have been tickets from users using Sinon.JS as more powerful timers implementation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok this seems reasonable if we want to support other fake timers than the ones from Jest. I'm curious though is there a way in user-event to configure the advanceTimers option globally or does it need to be done every time in the setup function?

Copy link
Member Author

Choose a reason for hiding this comment

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

For global, RTL recommends putting User Event setup into custom render function:

// setup function
function setup(jsx) {
  return {
    user: userEvent.setup(),
    ...render(jsx),
  }
}

test('render with a setup function', async () => {
  const {user} = setup(<MyComponent />)
  // ...
})

Source: https://testing-library.com/docs/user-event/intro#writing-tests-with-userevent

@mdjastrzebski
Copy link
Member Author

@pierrezimmermannbam I've discovered that RTL offers direct access to User Event functions (e.g. userEvent.press()). This is optional API:

Note that, while directly invoking APIs such as userEvent.click() (which will trigger setup internally) is still supported in v14, this option exists to ease the migration from v13 to v14, and for simple tests. We recommend using the methods on the instances returned by userEvent.setup().
Source

I've added that to this PR.

@codecov
Copy link

codecov bot commented May 9, 2023

Codecov Report

Patch coverage: 97.20% and project coverage change: +0.02 🎉

Comparison is base (a29a8c1) 96.86% compared to head (6a65d53) 96.88%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1405      +/-   ##
==========================================
+ Coverage   96.86%   96.88%   +0.02%     
==========================================
  Files          51       65      +14     
  Lines        3410     3693     +283     
  Branches      507      538      +31     
==========================================
+ Hits         3303     3578     +275     
- Misses        107      115       +8     
Impacted Files Coverage Δ
src/user-event/utils/events.ts 85.18% <85.18%> (ø)
src/fireEvent.ts 100.00% <100.00%> (ø)
src/test-utils/events.ts 100.00% <100.00%> (ø)
src/test-utils/index.ts 100.00% <100.00%> (ø)
src/user-event/event-builder/common.ts 100.00% <100.00%> (ø)
src/user-event/event-builder/index.ts 100.00% <100.00%> (ø)
src/user-event/index.ts 100.00% <100.00%> (ø)
src/user-event/press/index.ts 100.00% <100.00%> (ø)
src/user-event/press/press.ts 100.00% <100.00%> (ø)
src/user-event/setup/index.ts 100.00% <100.00%> (ø)
... and 5 more

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@pierrezimmermannbam
Copy link
Collaborator

@pierrezimmermannbam I've discovered that RTL offers direct access to User Event functions (e.g. userEvent.press()). This is optional API:

Note that, while directly invoking APIs such as userEvent.click() (which will trigger setup internally) is still supported in v14, this option exists to ease the migration from v13 to v14, and for simple tests. We recommend using the methods on the instances returned by userEvent.setup().
Source

I've added that to this PR.

@mdjastrzebski they support direct access but it seems to be only to ease migrations right? They recommend the usage of the methods returned by setup function so wouldn't it be best to have that as a single option so that we don't introduce breaking changes later on?

@mdjastrzebski
Copy link
Member Author

Tbh this seems relatively minor. If TL/UE decides to remove this API that would be a breaking change, which would force a major release.

Triggering a major release just for removing this API seems unlikely, more probably they would introduce some other breaking changes as well. That would probably require us to also increase the major version in order to remain compatible, which we could use to also remove the same API, in addition to bringing in other changes.

@pierrezimmermannbam
Copy link
Collaborator

Ok yeah that makes sense, all good for me then!

@mdjastrzebski mdjastrzebski merged commit dab444d into main May 10, 2023
@mdjastrzebski mdjastrzebski deleted the feat/user-event-core branch May 10, 2023 08:33
@mdjastrzebski
Copy link
Member Author

This PR has been released in v12.1.3 🚢
It is not added to package-level exports yet, waiting for userEvent.type() PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants