-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
Vscode jest monorepo support #4572
Vscode jest monorepo support #4572
Conversation
cc @orta |
Codecov Report
@@ Coverage Diff @@
## master #4572 +/- ##
==========================================
+ Coverage 55.61% 55.63% +0.01%
==========================================
Files 186 186
Lines 6354 6347 -7
Branches 3 3
==========================================
- Hits 3534 3531 -3
+ Misses 2819 2815 -4
Partials 1 1
Continue to review full report at Codecov.
|
}); | ||
|
||
describe('Terse Messages', () => { | ||
let parser: TestReconciler; | ||
// let results: TestFileAssertionStatus[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left over comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
fails: Array<TestFileAssertionStatus>; | ||
passes: Array<TestFileAssertionStatus>; | ||
skips: Array<TestFileAssertionStatus>; | ||
fileStatuses: {[key: string]: TestFileAssertionStatus}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice work 👍
// the processed test results will be returned immediately instead of saved in | ||
// instance properties. This is 1) to prevent race condition 2) the data is already | ||
// stored in the this.fileStatuses, no dup is better 3) client will most likely need to process | ||
// all the results anyway. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I think this is a better idea - good job 👍
"typescript": "^2.5.3" | ||
}, | ||
"devDependencies": { | ||
"typescript": "^2.5.3" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TypeScript is now in both deps + dev deps, I think it's fine in just deps 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that was dumb, fixed
verifyTest('passed', 'KnownSuccess'); | ||
}); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests are 🥇
Nice work, looking forward to seeing this in vscode-jest. |
* added watch mode and test_reconciler changes + upgrade typescript version * upgrade typescript (jest-test-typescript-parser) * clean up
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. |
Summary
Added the following changes to provide more reliable support for monorepo projects for vscode-jest:
test_reconciler. updateFileWithJestStatus()
instead of storing them as instance-variables (mainly to avoid race-condition for subsequent runs, bonus of reducing duplicate data).mainly to support a monorepo projects environment that needs to have an overrall view of all the projects, not just the one changed. see #129 for more detail.
Test plan
Note
since the test_reconciler has a non-backward compatible API change, not sure if there is any 3rd party package, other than vscode-jest, would be impacted. What is the guideline for changes like that... please advise.