search

projen / projen

Rapidly build modern applications with advanced configuration management
https://projen.io
Apache License 2.0
2.53k stars 362 forks source link

feat(typescript): adds `projencodedir` to jest testMatch patterns #3637

Open dkershner6 opened 2 weeks ago

dkershner6 commented 2 weeks ago

Fixes https://github.com/projen/projen/issues/3630

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter commented 2 weeks ago

Codecov Report

Attention: Patch coverage is 94.52055% with 4 lines in your changes missing coverage. Please review.

Project coverage is 96.36%. Comparing base (ad20d2c) to head (f5fd5c3). Report is 116 commits behind head on main.

Files Patch % Lines
src/javascript/jest.ts 92.98% 4 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #3637 +/- ## ========================================== + Coverage 96.34% 96.36% +0.01% ========================================== Files 192 194 +2 Lines 37696 38137 +441 Branches 3524 3574 +50 ========================================== + Hits 36320 36752 +432 - Misses 1376 1385 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

dkershner6 commented 2 weeks ago

Had to change the field from private to public on ProjenrcTs, and store projenrcTs on TypeScriptProject, and change the order of the constructor to get there, but everything seems as it should now, @mrgrain .

dkershner6 commented 2 weeks ago

As I was diving into the new change, I realized there were several very similar dirTestMatch pattern creations going on (and I was creating one more), so I centralized the logic to the Jest class in a static method, made it use the Jest best practices (from the docs and defaults), then used that everywhere, including the new feature.

So, it's a bit more than I intended to take on, but should now work really well and the responsibilities are in the correct places (and should be easier to maintain that in the future). @mrgrain

dkershner6 commented 1 week ago

Another nudge for @mrgrain .

mrgrain commented 1 week ago

Still on my list. Should get to it this week. Sorry for the delay.

dkershner6 commented 1 week ago

Still on my list. Should get to it this week. Sorry for the delay.

No worries, just wanna be sure it wasn't forgotten.

dkershner6 commented 4 days ago

@mrgrain , this has been updated.

In order to accommodate the discover method, I had to lazily load the testMatches. I am reasonably sure this actually fixed a bug where all testMatches added after initialization were ignored (but I don't know why all tests were passing, as this appears to be tested, somehow using the discover method seems to be different ordering, triggering defaults), but it does require a ts-expect-error due to types.