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 · 1 comment
Open

[RFC] Upstream Alignment Reference Questions #2

crutchcorn opened this issue Nov 8, 2021 · 1 comment
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).

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

2 participants