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

[jest-editor-support]: jest 23.x compatibility #6586

Merged
merged 2 commits into from
Jul 1, 2018

Conversation

connectdotz
Copy link
Contributor

Summary

This PR mainly to address jest 23.x compatibility and config parsing issues:

plus some clean up and self-diagnosis:

  • update the typescript typings
  • added a debug flag in workspace

Test plan

  • added new unit tests
  • test vscode-jest:
    • with jest 23.x repo, as well as non 23.x (backward compatible)
    • with CRA js/ts repo to make sure settings are extracted properly.

would also like to request @orta, @seanpoulter or @stephtr to review this PR.

Copy link
Member

@orta orta left a comment

Choose a reason for hiding this comment

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

O, yeah, cool, this looks good to me - yep. I've taken a stab at fixing CI in #6587

@stephtr
Copy link
Contributor

stephtr commented Jun 30, 2018

I didn't notice any obvious problems, but my insight into Jest 23 and jest-editor-support is much lower than in vscode-jest.

@connectdotz
Copy link
Contributor Author

connectdotz commented Jul 1, 2018

thanks @orta , @stephtr

@orta, looks like #6587 did pass the test-node-6 but somehow failed in continuous-integration/travis-ci/pr, not sure if they are related, do you plan to merge #6587 anyway? or should I just copy your fetchSupporters.js? or we can temporarily ignore test-node-6 as it is not related to this PR?

@orta
Copy link
Member

orta commented Jul 1, 2018

OK, I got merged #6587 - but that doesn't unbreak the build, just moves that fail a bit further up, so I'm going to merge this 👍

@orta orta merged commit d787040 into jestjs:master Jul 1, 2018
@connectdotz
Copy link
Contributor Author

@cpojer @SimenB is it possible to cut an alpha release so we can unblock vscode-jest users with jest 23.x dependency asap?

@connectdotz connectdotz deleted the jest-editor-support-parsing-fix branch July 3, 2018 12:56
@thymikee
Copy link
Collaborator

thymikee commented Jul 3, 2018

cc @mjesun

@mjesun
Copy link
Contributor

mjesun commented Jul 3, 2018

I can make a patch, if @thymikee and @SimenB agree.

@SimenB
Copy link
Member

SimenB commented Jul 3, 2018

I'd like to rename the prettier option for inline snapshots. Can send a PR within the hour for it. Then we should be good to go :)

Ref #6380 (comment)

@SimenB
Copy link
Member

SimenB commented Jul 3, 2018

#6607

@SimenB
Copy link
Member

SimenB commented Jul 3, 2018

Oh, and #6603. CI is weird, should be good to land (I restarted the two CIs)

@mjesun
Copy link
Contributor

mjesun commented Jul 3, 2018

@SimenB 👌🏻 Feel free to merge all three (I think you have permissions) and I'll then proceed. This looks however like a minor, though. Are you OK with that?

@SimenB
Copy link
Member

SimenB commented Jul 3, 2018

Yeah, it's a minor. Will merge and keep a screenshot of your comment if @cpojer comes after me with a torch! 😀

@SimenB
Copy link
Member

SimenB commented Jul 3, 2018

@mjesun should be ready for release now 🙂

@connectdotz
Copy link
Contributor Author

thanks for everybody's effort! I checked the release log this morning and it was still at 23.2.0. Is there some other place I should check for the new release?

@mjesun
Copy link
Contributor

mjesun commented Jul 4, 2018

@connectdotz was a busy morning, sorry! It's released now :)

@connectdotz
Copy link
Contributor Author

awesome! 👍

let settings = null;

try {
settings = JSON.parse(text);
Copy link
Contributor

@stephtr stephtr Jul 12, 2018

Choose a reason for hiding this comment

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

By the way: When starting up your vscode-jest branch with a CRA app, I always get a "failed to parse config" with text being just (that's the additional output of npm test -- --showConfig):

"
> cra-ts-test@0.1.0 test c:\Users\stroy\Projekte\_Playground\cra-ts-test
> react-scripts-ts test --env=jsdom "--showConfig"
"

However since _parseConfig also gets called with the real settings afterwards, the extensions work without flaw.

@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants