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

Should test configuration setupTests.{ts,js} be per-application? Should tests be run globally against the project or are they run against specific applications? #103

Closed
sebinsua opened this issue Sep 18, 2020 · 5 comments · Fixed by #107

Comments

@sebinsua
Copy link
Contributor

sebinsua commented Sep 18, 2020

Should test configuration setupTests.{ts,js} be per-application?

I noticed that CRA would only load the setupTests.{js,ts} of one application and amended our test configuration to load the test configuration of each application. This was done because I didn't want any one application to be special-cased as containing the configuration for all other application's tests, however since all of the setupTest files are loaded when the tests are run, it means that if you make a change to the configuration you do so for everybody. There could be conflicts.

Should tests be run globally against the project or will they be run against specific applications?

I thought the former was what we were trying to achieve, and if that's the case and presuming that others agree that distributing configuration across a repository is a Bad Idea™, then it seems like a good idea to move to having a single setupTests.{js,ts} at the root of the modular project.

However, it's also possible that every application could have their own modular test [app-name] command that they run. This would certainly be helpful when migrating applications with differing test configuration into a single repository. Additionally:

  • Are there long-term benefits to making modular test work this way?
  • What about packages and widgets that aren't part of the application, but are depended upon by it -- how would we know that the tests for these should be run with a particular application's modular test [app-name] command and that its setupTests configuration will run these correctly?
  • When you make a change to a design system that affects all application's within a single repository, what test command would you use to make sure nothing is broken -- presumably there would still need to be a modular test command without an [app-name] argument that can run all of the tests and that would need to be configured with every single setupTests.{ts,js} within the repository?

See also: #100 (comment)

@sebinsua sebinsua mentioned this issue Sep 18, 2020
2 tasks
@threepointone
Copy link
Contributor

Thinking about this more, I might have changed my mind. Perhaps a single setupTests for the entire repo is fine.

Honestly, I think will be answered when we try to add multiple applications to a single repo and see what assumptions break. Probably none.

@threepointone
Copy link
Contributor

Where should setupTests.js be? Root?

@sebinsua
Copy link
Contributor Author

sebinsua commented Sep 18, 2020

Perhaps a single setupTests for the entire repo is fine.

Yeah, I think so, too. Well, I think it'd be the best goal at least...

I think the problem will be when migrating tests into one project. If two different applications have different Enzyme adapters, monkey-patched mocks of fetch, or some other test setup that is not going to be compatible then we might need some way to have separate configuration for different test runs, and to make sure that the correct test setup is run for certain test files...

I'm wondering how far we could get with something like:

${modularRoot}/setupTests/index.js:

// Global setupTests stuff goes here...

if (process.env.TEST_APP === 'SOME_SPECIFIC_APP') {
  require('./some-specific-app/setupTests');
}

This might be enough in some cases, and perhaps in some cases you could remove the environment variable and run all of the configuration for all apps, but if there is incompatability then you would need a way to switch the environment variable for only some of the test files, etc...

Generally, we'll probably need people to rewrite tests in order that they're not dependent on any configuration specific to their app...

Does anybody know a way of only running something before some test files?

@threepointone
Copy link
Contributor

Honestly, maybe that’s a good place to draw a line in the sand. They’re welcome to unmock/remock in individual files, and if it’s a lot, they can codemod the boilerplate. Let’s strive for a singular setupTests for now, and play it by ear. Should be an enlightening exercise.

@sebinsua
Copy link
Contributor Author

sebinsua commented Sep 21, 2020

I'll make this change this week.

What I'm tempted to do is to leave the logic which loads packages/*/src/setupTest.{js,ts} since this is CRA-like (and their docs would tell people to put setupTests files here -- the other possibility I guess is that I could console.warn about this). But, importantly, I would copy the default setupTests.{js,ts} file into the modular root by default and document that this is the correct location.

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 a pull request may close this issue.

2 participants