Skip to content

CI variable used for both single test run and treating warnings as errors #3925

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

Closed
mattmazzola opened this issue Jan 27, 2018 · 5 comments
Closed

Comments

@mattmazzola
Copy link

Background:

When set to true, Create React App treats warnings as failures in the build. It also makes the test runner non-watching. Most CIs set this flag by default.

https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/template/README.md#advanced-configuration

By default npm test runs the watcher with interactive CLI. However, you can force it to run tests once and finish the process by setting an environment variable called CI.

https://github.com/facebook/create-react-app/blob/master/packages/react-scripts/template/README.md#continuous-integration

Info:
I would like to be able to manage these features independently so I may have single test run in Travis but not treat warnings as errors. However, both are set to true because CI is enabled by default

Is there a way to specifically toggle one of these parameters? Perhaps a custom flag I can pass to jest test runner through npm script?

@gaearon
Copy link
Contributor

gaearon commented Jan 28, 2018

Can you explain more about your use case?

@mattmazzola
Copy link
Author

Use case is trying to get CI build for PR's running successfully without first having to fix all the tslint warnings.

I would understand it's good to keep the defaults as best practices and treating these warnings as errors, but I was hoping there was something extra I could configure to avoid this.

If I remove/unset CI environment variable this would prevent treating the warnings as errors, but would make the test command go into watch mode and block

As it is now it seems I have 2 options:

  1. Remove all the tslint rules:
    Would like to preserve them so we can see them and work towards fixing them.

  2. Fix all the warnings:
    Perhaps we could alleviate this by auto-correcting some easy rules like quotes or enabling Prettier

I was hoping for a 3rd options which would be middle ground which allows to keep tslint rules, but not fail the build while still having jest configured for single test run.

If you think it's too risky to allow production builds to run with warnings not as errors then I'll likely go with one of options above, but I assumed someone else has come across this conflict and there would be a way to configure this. Perhaps in the .travis.yml I could do something like:

script: npm test -- --singleRun to over ride the default npm test command which seemed to have --watch passed through react-scripts

@Timer
Copy link
Contributor

Timer commented Jan 29, 2018

tslint

This sounds like you're using react-scripts-ts, which means you could ask for this feature on their fork (since you're not our package, react-scripts).

Assuming they haven't diverged much, this should work in the mean time:

cross-env CI=false npm run build

@mattmazzola
Copy link
Author

mattmazzola commented Jan 29, 2018

Yes, I am using react-scripts-ts which I believe is from here:
https://github.com/wmonk/create-react-app-typescript

you could ask for this feature on their fork

Ok. Yea I think we can close this one then. I didn't realize they diverged as much in behavior. (was thinking they only swapped eslint for tslint and babel for typescript, but it seems that this package doesn't have same type of linting by default which means treating these warnings as errors is not relevant here.

I will try the suggested script.
From my understanding this removes CI environment var for the build script making it ignores warnings; however, for the next script (npm test) CI is still globally enabled so jest will do single run.

@Timer
Copy link
Contributor

Timer commented Jan 29, 2018

From my understanding this removes CI environment var for the build script making it ignores warnings; however, for the next script (npm test) CI is still globally enabled so jest will do single run.

Correct, which is the behavior you desired. 😄 (this works on react-scripts, and hopefully the ts fork)

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants