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

Start Jest with the --no-color CLI argument #293

Merged
merged 1 commit into from
Aug 7, 2018
Merged

Start Jest with the --no-color CLI argument #293

merged 1 commit into from
Aug 7, 2018

Conversation

seanpoulter
Copy link
Member

This PR will prevent the ANSI escape codes from appearing in the test messages. This is dependent on a change to jest-editor-support that adds the --no-color option (jestjs/jest#5909).

@coveralls
Copy link

coveralls commented Apr 2, 2018

Pull Request Test Coverage Report for Build 427

  • 4 of 4 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 84.007%

Totals Coverage Status
Change from base Build 426: 0.0%
Covered Lines: 690
Relevant Lines: 806

💛 - Coveralls

@orta
Copy link
Member

orta commented Apr 2, 2018

I’ve merged the other, but this now relies on a new jest release

@seanpoulter
Copy link
Member Author

We're still waiting for a Jest deployment. It's making me wish we could go bump jest-editor-support on it's own.

@orta
Copy link
Member

orta commented May 19, 2018

There's nearly always a jest release: https://www.npmjs.com/package/jest (see versions)

@stephtr
Copy link
Member

stephtr commented May 21, 2018

Switching to a beta release of jest-editor-support (for example 23.0.0-charlie.2 as done within #297) should work (since it doesn't contain any other beside our own changes).

@seanpoulter
Copy link
Member Author

I wonder if we can fix this issue now without the option. 🤔

@stephtr
Copy link
Member

stephtr commented May 22, 2018

You mean without the --no-color option?

@seanpoulter
Copy link
Member Author

I wonder if we can fix this issue now without the option. 🤔

On second thought it's not worth any thought.

Switching to a beta release of jest-editor-support ... should work

Do we need to worry about breaking anything else? Things aren't very stable right now ... 🔥

@stephtr
Copy link
Member

stephtr commented May 26, 2018

I can't think of any new instability since there had been only a handful of changes (mostly made by us) which I think only introduced new features. I already tested it with my PR and didn't notice any issues.
But since Jest 23 has been released on Thursday we probably need some testing anyhow.

Copy link
Collaborator

@connectdotz connectdotz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@seanpoulter once we merge #341, we should be in sync with the latest jest-editor-support that this PR depends on.

The change is small and simple, I see no problem in merging in, just need to resolve the merge conflict as there are many PRs went ahead of this one...

@seanpoulter
Copy link
Member Author

Was there something about Jest 23 that solved this problem for us? I don't remember ...

@connectdotz
Copy link
Collaborator

connectdotz commented Jul 20, 2018

the new option noColor depends on the the changes in jest-editor-support jest/5909 you submitted a while back. Not sure which exact jest version includes that change, but with jest 23.3 (updated in #341) I know for sure it is in there.

During #341, I briefly tested the noColor option and it seems to work, however I have only tested with jest 23.x. I think as long as it won't break other jest versions, we should be fine.

@seanpoulter
Copy link
Member Author

OK, thanks for refreshing my memory. I'll rebase that when I get a few moments.

@seanpoulter
Copy link
Member Author

I've rebased the PR which should be good to go @connectdotz.

@connectdotz connectdotz merged commit ded6a14 into jest-community:master Aug 7, 2018
@connectdotz
Copy link
Collaborator

thanks @seanpoulter

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.

5 participants