javascript: Fix failing test and make script/test exit non-interactively - #154
javascript: Fix failing test and make script/test exit non-interactively#154adamnfish-gu wants to merge 1 commit into
Conversation
pairingTest() returned false while the test expected true. It now returns true, and the test is named for what it checks. package.json ran `jest --watchAll` and script/test added --watch, so the runner never exited. package.json now runs plain jest and script/test takes an explicit --watch flag. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| const { pairingTest } = require('.'); | ||
|
|
||
| test('a failing test', () => { | ||
| test('pairingTest returns true', () => { |
There was a problem hiding this comment.
the intent here seems to have changed, as well as the behaviour
it looks to me as though originally this was meant to be a failing test, along the lines of "Red-Green TDD", to verify that the test setup is actually working.
if we're adding a CI step that runs the tests then this is a good reason to switch to using a passing test rather than a failing one, but as this goal isn't mentioned in the PR description I thought it worth flagging
There was a problem hiding this comment.
Thanks for the review. As also mentioned here, that's very true. We're switching to the skeletons having passing tests so we can use that to validate the setup.
I've added a section to the over-arching Issue's text entitled "Changes to the test-suites" to make this clearer.
Thanks heaps for calling out that this wasn't clear enough.
2 participants
pairingTest() returned false while the test expected true. It now returns true, and the test is named for what it checks.
package.json ran
jest --watchAlland script/test added --watch, so the runner never exited. package.json now runs plain jest and script/test takes an explicit --watch flag.Base
platform/tool-versions-ci(#144), so the diff shows only this branch's changes. Merge #144 first.Part of #143.