-
Notifications
You must be signed in to change notification settings - Fork 89
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
Run project tests as part of template testing #623
Run project tests as part of template testing #623
Conversation
Larocceau
commented
Jul 12, 2024
- Add build action in default project to run tests one-off using mocha test runner (instead of in browser)
- Add build action in template build script to run these tests
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.
This looks like a good step in the right direction 🙂 I've made a few suggestions
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.
Maybe to make the distinction clearer, we should call these targets "RunTestsHeadless" and "WatchRunTests"?
If we rename "RunTests" we should also update docs, so I'm happy for that to be split into a separate issue/PR rather than done now.
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.
did the rename for runTestsHeadless, let's do the other change next week in conjunction with a docs update
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.
Thanks! Could you raise an issue to make sure that we remember to do that?
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.
Let's make sure to update the docs too 👍🏼
Co-authored-by: Matt Gallagher <[email protected]>
Co-authored-by: Matt Gallagher <[email protected]>
31e1f34
to
60a9a97
Compare
All feedback dealt with |
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.
Happy for this to be merged so long as the docs are updated to reflect the changed target name
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.
Oh, one last thing: the template's README needs updating to reflect the changed target name.
My last comment is wrong - the README had already been updated. |