Closed TimKnight-DWP closed 8 months ago
Attention: 2 lines
in your changes are missing coverage. Please review.
Comparison is base (
16be049
) 97.65% compared to head (1d799a0
) 94.42%.
Files | Patch % | Lines |
---|---|---|
jest.config.js | 0.00% | 2 Missing :warning: |
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I was trying to work out why it failed but didn't have any output, and then saw the --silent
😅
Heh - yeah, there was a really weird bug in the mocking of gitSemverTags for how the callback was working 🙈
And because of some weirdness in clearing out the stderr logs, some of the tests failed due to earlier tests spitting something naff into the logs
I know this isn't passing yet, but it's looking really nice - it seems like a clear readability improvement to me.
Thanks, I feel like I'm inching towards getting git tests working, thought doing core.spec would have been the hardest, but run into a subtle difference between looking at stdout.error compared to spying on console (in that console.warn ends up in stdouterror which threw me off for a bit when trying to look in console.error calls for certain logs)
@TimothyJones - okay that should be pretty much done, going to come back with fresh eyes in the morning and check there aren't any verifications I can improve now I have a better understanding of testing exceptions and errors printed to console.warn
Awesome! I’ll review this evening (in about 8 hours) and we can get this in. Thank you so much!
Awesome - sounds like my morning will roughly align with your evening 👍
Removed the upper limit and now run tests always against the current lts and latest versions published by node - should give us more heads up of breakages, working versions, with less manual effort to add more checks for specific versions
Right - all assertions/verification are the same or as close as possible to where they were before. And running on LTS (20.9) and Latest (21)
Good to go 👍
fyi I won't be in on this again til Monday, in case you've got any change requests, just to set expectations so you know not to look at it after comments until next week 😄
Fixes #118
Because mock-fs creates a fake Filesystem in memory I had to do a lot of changes to core.spec.js to re-work both the mocking and verification to make use of Jest spies instead.