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

[RFC] Upstream Alignment Reference Questions #2

Open
crutchcorn opened this issue Nov 8, 2021 · 3 comments
Open

[RFC] Upstream Alignment Reference Questions #2

crutchcorn opened this issue Nov 8, 2021 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@crutchcorn
Copy link
Owner

crutchcorn commented Nov 8, 2021

As should be obvious, we're trying to match the upstream example of Testing Library ecosystem as closely as possible. This even extends so far as our source code and release/dev tooling.


However, there are some existing APIs in related packages that I'm admittedly not sure how they would fit/migrate into the CLI environment

General Questions

Library Merging

Currently, this library does the job that the broader Testing Library ecosystem would usually split into a few different packages:

On top of this, we also have our docs stored in this folder, whereareas upstream has a dedicated repo and website.

Theoretically, we could break these into separate libraries but there's a few problems with this:

  1. Without a specific org, it'd be difficult to see all the relevant repos in my personal namespace on GitHub. While I'm not against making an org, ideally we'd be able to merge upstream into the Testing Library org instead.
  2. While dom-testing-library and dependents have the benefit of relying on W3C web standards, we do not. The only standards present for the CLI are, well, the CLI itself. There is many instances where, unfortunately, logic is interconnected by requirement. It makes little/no sense to split them when their logic is so interconnected

Instance

While we have an implementation present, it's unclear what a TestInstance properly should be, or if that should even be it's name.

We've done our best; for example, to align with container of upstream, we have the return result of spawn set to container in CLI Testing Library.

However, because there is no distinction between stdout and stderr outputs (oftentimes a CLI app will output multiple items one-by-one to be visually similar to one-another), there's no way to have the same kind of parent/child relationship that the DOM has. This has many unintended consequences that we'll see shortly.


Further, what should we be calling TestInstance? They're meant to be a replacement to HTMLElement and returned by RenderResult.

Here are some alternatives I can think of:

  • Process (not a huge fan of this one)
  • CliInstance
  • CommandLine
  • Console

Window

Something that I think comes in handy, from a library perspective, is a global that keeps track of all running processes, like window in the DOM.

While it wasn't needed in v1 (quite yet), it will make some refactor work possible. As such, I want to include it in v2.

Two questions related to that.

1: What should we call it?

  • Machine (I like this one TBH)
  • OS
  • Kernel

2: What should the shape look like?

window has many helper methods, and such. We could do something like:

{
    // `kill` all processes
    shutdown(): void;
    // `kill` PID from `prosseses`
    kill(pid: number): Promise<void>;
    processes: TestInstance[];
}

dom-testing-library Reference Questions

Queries

While we have singular queries (such as findByText and findByError), we do not currently have multiple matcher queries.

  • queryAllBy
  • findAllBy
  • getAllBy

This is because, as mentioned before, there is no reference for what these should return. There are no distinctions between stdout or stderr lines, and there's no major parent/child relationship I can think of that would make sense.

jest-dom Reference Questions

While we have some matchers similar to jest-dom (EG: toBeInTheConsole), would it also make sense to add something like:

  • toHaveANSIEscapeModifier - Test to see if a related ANSI Escape Code applied to the line
@crutchcorn crutchcorn added the help wanted Extra attention is needed label Nov 8, 2021
@Waldeedle
Copy link
Contributor

@crutchcorn are you still looking for comments on this RFC?
In my mind, I would base the structure of this package based on the goals and also long-term maintainability.
I would think that users of this project wouldn't use partial components of it, i.e. using the Jest matchers for the cli.
With that in mind as well as the note about it being interconnected, I personally would go with the approach that makes your life easier (having all relevant code in one repo).

@mdjastrzebski
Copy link
Contributor

Hi @crutchcorn, first off, huge congrats on creating such an awesome library! 🎉

I wanted to jump into this discussion as the main maintainer of [React Native Testing Library(https://github.com/callstack/react-native-testing-library). I’ve noticed many similarities between our libraries as non-Web members of the Testing Library family, and I think there’s a lot of value in sharing ideas and experiences.

Library merging

As far as understand the React Testing Library started as integrated one, with DOM Testing Library and Jest DOM matchers being extracted later on as other TL were added: Angular, Svelte, etc.

In case of RNTL we analysed this case and came to the conclusion, that as a non-Web Testing Library, which needs own implementation of Jest Matchers, User Event, etc It makes more sense to have this integrated inside our core package: @testing-library/react-native.

The main pros we found:

  • easier maintenance and testing for library maintainers
  • keeping shared logic in the same repo (queries and matchers)
  • ensures that user always have matching versions
  • no alternative consumer of RN-specific Jest Matchers & User Event packages

On the cons side it was only: difference from RTL/DTL structure which users might be familiar to.

We event made effort to re-integrated the previously existing @testing-library/jest-native matchers back into main @testing-library/react-native package (callstack/react-native-testing-library#1468).

@crutchcorn
Copy link
Owner Author

Hey @mdjastrzebski! This is so cool to hear from y'all over Callstack - big fan of your work having led a RN shop as recently as last year.

I came to much of the same conclusion. I:

There's likely more work I could do to extend out this package, but today it serves my needs well powering E2E tests in Plop, although feature requests and such are more than welcomed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants