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

Fix failing tests #28

Merged
merged 2 commits into from
Sep 14, 2021
Merged

Conversation

msabramo
Copy link
Contributor

@msabramo msabramo commented Sep 13, 2021

The GitHub Actions CI has been failing for a while because the output is not rendering colored. This is because test.js was setting FORCE_COLOR=1 but this happens after the import of chalk and thus also after the import within chalk of supports-color

JS likes imports to be at the top of files before any other logic, so it's not really possible to set FORCE_COLOR any earlier within test.js. But it is possible to set it in package.json while invoking ava...

Screen Shot 2021-09-13 at 4 27 18 PM

Fixes: GH-27

This PR also removes node versions 8 and 6 from the list of versions to test, because yargs-parser only supports node >= 10 now. Note that we go forward with converting chalk-cli to ESM (see #26), then we will require node >= 12.

Screen Shot 2021-09-13 at 4 25 52 PM

🎉

The GitHub Actions CI has been failing for a while because the output is
not rendering colored. This is because `test.js` was setting
`FORCE_COLOR=1` but this happens after the `import` of `chalk` and thus
also after the import within chalk of
[`supports-color`](https://www.npmjs.com/package/supports-color)

JS likes imports to be at the top of files before any other logic, so
it's not really possible to set `FORCE_COLOR` any earlier within
`test.js`. But it *is* possible to set it in `package.json` while
invoking `ava`...

Fixes: chalkGH-27
These versions of Node no longer pass tests, I think because
[yargs-parser only supports node >= 10](
https://github.com/yargs/yargs-parser/blob/main/package.json#L79-L81):

```js
  "engines": {
    "node": ">=10"
  },
```
@sindresorhus sindresorhus merged commit 8caedad into chalk:main Sep 14, 2021
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 this pull request may close these issues.

Fix failing CI tests
2 participants