-
Notifications
You must be signed in to change notification settings - Fork 208
feat: support comments in JSON #571
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
Conversation
Is there a way I can assist this get merged? GitHub says that a test has failed. Unfortunately, the logs have expired, so we don't know what the problem was. |
33a2896
to
a7b5eb0
Compare
This is the weirdest thing. The tests always pass locally. They fail sporadically, not dependent on node version, in GitHub Actions with the error:
@pastelmind if you're interested, you could always fork the repo, checkout this branch into your own fork, and then submit a PR to try and figure things out :) Just make sure to retry the tests a few times, because the error seems to be flaky. |
After closer inspection.... I forgot to |
@JustinBeckwith apologies for dropping this review on the ground, mind cleaning up the |
@@ -49,6 +49,24 @@ describe('clean', () => { | |||
}); | |||
}); | |||
|
|||
it('should gracefully handle JSON with comments', () => { |
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.
I think that these assertions should be async too? () => {
to async () => {
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.
Nope! They return a promise, since withFixtures
returns a promise, which seems to be what mocha is actually looking for. All the tests in this file seem to work like that 🤷
Fixes #442