-
Notifications
You must be signed in to change notification settings - Fork 33
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(packages/sui-jest): Allow test server& browser environments #1655
base: master
Are you sure you want to change the base?
Conversation
|
||
program | ||
.option('-W, --watch', 'Run in watch mode') | ||
.option('--ci', 'Run a Firefox headless for CI testing') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There isn't a plural version for Firefox. This should be one of the following:
1 - Run a Firefox headless browser for CI testing
2 - Run Firefox in headless mode for CI testing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I should check/update this description in order this is wrong I guess. Thanks again for detect it!
|
||
program | ||
.option('-W, --watch', 'Run in watch mode') | ||
.option('--ci', 'Run a Firefox headless for CI testing') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment for the Firefox pluralisation also applies here
|
||
// TODO: Decide which is the default common config | ||
const jestConfig = { | ||
// All imported modules in your tests should be mocked automatically |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should remove all the commented options if they are not used. We can always read the official docs for the available API.
config = {...config, ...projectJestConfig} | ||
} | ||
|
||
args.push(...['--config', JSON.stringify(config)]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need to create a temporary array for the spread operation. [].push(arg1, arg2, arg3)
works with comma separated arguments.
|
||
args.push(...['--config', JSON.stringify(config)]) | ||
|
||
require('jest').run([...args]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use only require('jest').run(args)
? There is no need for the temporary array that is used for the spread operation. The args
array is also available only inside the scope of the function, therefore, it can't be modified globally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! 👏🏻 👏🏻 👏🏻
@@ -0,0 +1,29 @@ | |||
#!/usr/bin/env node | |||
/* eslint-disable no-console */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed ?
Nice 👏🏻 |
Hello, excellent iteration. If you allow me, I would like to share a few more of the design context of IMO, projects don't have the same needs; an example is within a monorepo as the example you are sharing:
In this way, the Regarding differentiating
If there is, for a test, the need or use case the difference of being executed on a And as the last point to give context, my intention with I hope that with these thoughts, I can help you decide where we want to go, simply, both paths will be accepted for me 🚀 |
First of all, thanks for your top explanation @giodelabarrera ❤️ . Now, I've more clear why If I'm not wrong, the purpose of SUI is to wrap and standardise as much as possible through all projects that uses SUI; and empower them to add little customisations. For example:
I see your point to give each project the 100% of the configuration to work, but maybe it would be nice to standardise it from scratch by defining a base configuration. Or, if it's needed, have different configuration for I'll answer you by topic, in order to improve the current approach without rewriting as I did:
That's great! We missed this important point when we use it on our project. We have created a main So, ok, i'm agree with you. This challenge could be solved by creating a
Yes, by default For domain testing, we have two scenarios:
In resume, for those cases, we should:
As you have seen, by answering you I've purposed some how we can use the current implementation of I'm going to play a little but with those things and see possible new approaches. But I would like Thanks again for your feedback. |
Great job and contribution, @oriolpuig ! 👏 I understand @giodelabarrera 's point to allow for each project to have it's own configuration and a good thing this iteration brings is that it still allows for that while it gives a default config that could work for most if not all our projects. I think we could even have different configurations for different types of projects (ie domain, components...) by declaring so on the project's I may lack some context as I am not familiar to Jest and what problems it comes to solve that we couldn't solve before. All that said... again, good stuff, @oriolpuig ! :) |
Description
Current version of
sui-jest
is passing the responsibility of having a jest.config.js file to each project, instead of follow the usual SUI approach by wrapping a library and unify how as much as projects uses the same configuration.We have started using jest with component testing, but when we started to use it on
sui-domain
tests, we needed some customisations to run test on server and client sides. Those customisation, cause some extra project configuration that we can avoid with this Pull RequestWith this Pull Request I want to make our life more easier and standardise the default
sui-jest
configuration through all sui projects. Of course, we also allow each project to pass custom configuration and override the default one.Some things this Pull Request enables:
sui-jest browser
orsui-jest server
to run tests on different environments and it will add extra configuration needed for each environment..client.
or.server.
extension files.ts | tsx | js | jsx
files by default.sui-jest browser
will read files like:helloWorldUseCase.test.js
helloWorldUseCase.browser.test.js
helloWorldUseCase.client.test.js
helloWorldUseCase.test.ts
component.test.tsx
sui-jest server
will read files like:helloWorldUseCase.test.js
helloWorldUseCase.server.test.js
helloWorldUseCase.test.ts
Pending / TODO
sui-domain
playgroundtsx | jsx
files onsui-jest server
scriptjest.config.ts
file.Example