chore(tests): fail tests on console output #453
Conversation
| */ | ||
| require('./setup.common') | ||
|
|
||
| jest.spyOn(console, 'log') |
There was a problem hiding this comment.
I would like to confess that those changes are mine, but this is not the case :) actually, the approach of using spies for console methods was already introduced before and just have been removed from sources for a while
| @@ -0,0 +1,23 @@ | |||
| module.exports = { | |||
There was a problem hiding this comment.
actually, you are partially right, but let me provide full story behind that.
Currently ts-jest tool is responsible for transpiling jest tests before they will run - and this fact is stated by value of transform option of Jest config file. Thus, to run the tests we don't need to do any transpilation ourselves for test files as long as this tool is used for making necessary transform. Another thing to note is that ts-jest doesn't produce any (visible) transpilation artifacts - in contrast to the process that is used to compile sources.
For source files, in contrast, tsconfig.json file is in charge of defining how and which files should be transpiled.
Answering on your question - while, yes, transform option serves to suggest Jest that ts-jest should be used for making transform, there is no was to specify directly there that 'raw' TS compiler (without being wrapped as a plugin) should be used. If we would like to use raw TS compiler, we would need to make test process consisting of two steps:
- transpile test files and store them
- run tests for transpiled (JS) test files
To my mind this move would definitely complicate things, but let me first take a look on the issue to obtain more details on why this move might be necessary
As I remember, I had similar issues in Semantic-Org/Semantic-UI-React#2971. I think that we should wait for @levithomason there before merge. |
|
Will address the error so we can go ahead with the merge. I will not create any additional bug, as this PR should serve for tracking. |
we may wait, ok, but if it will take too long, it seems that we'd better address the problem that we clearly have now, rather than just let similar problems sneak through |
|
The issue with the Input test lays in the I know @Bugaa92 is working currently on a PR that will prevent spreading props in this case, so it may be fixed in his PR. For unblocking this one, I would suggest removing this test, and addressing it later if we decide that this is something we want to support. @kuzhelov @miroslavstastny what do you think? |
|
Btw, the usage |
|
@mnajdova, @Bugaa92, have introduced fix as part of this PR, thus will merge now |
Blocked by
Listcomponent: fix(List): remove listRef from API #489Inputcomponent issue (discovered by tests) - fix introduced as part of this PRThis PR aims to introduce CI gate checks that will verify that there is no console output logged during unit tests run.
Was
This feature was originally eliminated by the following PR (#359) - please, consider to review description where reasoning for this step is expressed. However, this move has resulted in the following problem sneaked into the code and being accidentally committed (please, note that test state is 'PASS'):
Now
With this PR the feature is restored while, at the same time, all the benefits of clear error output are preserved for local test runs. As a result, for local test runs there will be exactly the same behavior as it was before (with
yarn testorjestbeing used), while for CI build--strictflag will be applied:yarn test --strict.PS. Please, note that this PR will result in failed CI builds till the moment when aforementioned
Listissue will be addressed.