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

Allow setupTestFrameworkScriptFile to accept an array (or add a new config option that does) #6942

Closed
kentcdodds opened this issue Sep 4, 2018 · 16 comments · Fixed by #7119

Comments

@kentcdodds
Copy link
Contributor

🚀 Feature Proposal

I would like to pass multiple files to be run immediately before each test file is run but after the test framework has been loaded. This would either amount to extending setupTestFrameworkScriptFile to accept an array of files, or a new config option (setupTestFrameworkFiles?) to allow this.

Motivation & Example

I can change this:

// setupTestFrameworkScriptFile.js
import 'jest-dom/extend-expect'
import 'react-testing-library/cleanup-after-each'

Into this:

// jest.config.js
module.exports = {
  // ...
  setupTestFrameworkFiles: [
    'jest-dom/extend-expect',
    'react-testing-library/cleanup-after-each',
  ]
}

Pitch

This would simplify the project because the fact that setupFiles accepts an array and setupTestFrameworkScriptFile only accepts a string is confusing. It would also mean I don't have to create a file with two lines of code which would be nice. This would also make extending toolkit with a built-in setupTestFrameworkScriptFile with some custom behavior for the project.

@SimenB
Copy link
Member

SimenB commented Sep 4, 2018

I think it makes sense to add setupTestFrameworkFiles and deprecate the other to better match setupFiles. I also agree that keeping it in config is nicer.

@rickhanlonii @thymikee thoughts?

@thymikee
Copy link
Collaborator

thymikee commented Sep 4, 2018

Yup, agreed with @SimenB plan

@mattphillips
Copy link
Contributor

+1 to this proposal

I’m delighted to make this config option more useable and less verbose by dropping Script from the name, as that’s caught me out many of times 😆

@SimenB
Copy link
Member

SimenB commented Sep 4, 2018

Yeah, more consistency with setupFiles 🙂

@thymikee
Copy link
Collaborator

thymikee commented Sep 4, 2018

should we rename this to setupTestRunnerFiles to be more compatible with jest-circus being a testRunner??

@SimenB
Copy link
Member

SimenB commented Sep 4, 2018

Not sure, as it doesn't really modify/setup the runner itself. I could probably be convinced quite easily, though

@mattphillips
Copy link
Contributor

I wonder if we could dropFramework too?

Either setupTestFiles or testSetupFiles seem to reflect what is actually going on as these files are run before each test.

That said I appreciate this is subjective and could be bike shedding so happy to go with what you guys think 😉

@SimenB
Copy link
Member

SimenB commented Sep 4, 2018

All we need to clearly communicate is that setupFiles run before the test framework is injected, and setupTestFrameworkScriptFiles (or whatever it ends up being named) is run after. I don't think either name really does that ATM, but it is what it is

@natealcedo
Copy link

It seems like @SimenB is on the right track with intent needing to be conveyed accurately.

On the topic of setupFiles, the docs state
The paths to modules that run some code to configure or set up the testing environment before each test.

Why not rename this to setupTestEnvironmentFiles?

On the topic of setupTestFrameWorkScriptFile, docs state again
The path to a module that runs some code to configure or set up the testing framework before each test.

Why not rename this to setupTestFrameWorkFiles?

kentcdodds pushed a commit to kentcdodds/jest-axe that referenced this issue Sep 4, 2018
Must use `setupTestFrameworkScriptFile` ([hopefully this will change soon though](jestjs/jest#6942))
@SimenB
Copy link
Member

SimenB commented Sep 11, 2018

Upcoming major (because of babel 7), so we can just deprecate the old one if needed

@SimenB
Copy link
Member

SimenB commented Sep 11, 2018

setupTestsBeforeJest, setupTestsAfterJest? A bit verbose, but descriptive names are more important than short names

@kentcdodds
Copy link
Contributor Author

I'm good with those names 👍

Gonna have to re-record a few egghead lessons I'm working on :P But this is a great change!

@mattberg88
Copy link

I'd like to pick this up as my first contribution to the project.

@SimenB
Copy link
Member

SimenB commented Sep 12, 2018

Awesome, go ahead! Feel free to ask questions if you have any :)

@ddruker
Copy link
Contributor

ddruker commented Oct 8, 2018

@SimenB Decided to give this a try and just made a PR. Figured we can talk around the code on the PR.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants