Add example test for base.request.js #39
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `base.request.js` module in [water-abstraction-system](https://github.com/DEFRA/water-abstraction-system) is a great candidate for rewriting its tests using the node-test runner. In the module itself, we can bring in [Got](https://github.com/sindresorhus/got) using a standard import. In the tests, we bring in and [Nock](https://github.com/nock/nock) for the first time and confirm that works as before. We also deal with mocking our `GlobalNotifier` for the first time (we've not even added it yet!) and make assertions against it. The key things we're hoping to get to the bottom of though is - how would we alter the value of one of our config files, as we can do with `Sinon.replace()` - will we need to use the [timeout option](https://nodejs.org/api/test.html#testname-options-fn) like we do in Lab for our timeout tests Plus, as while we are working on a new test, we give the updated `--test-only` functionality a run for its money!
We've confirmed we can now place our `.only()` anywhere in the test file, and it will only run it, or those in its block (we don't have to place `.only()` at every level any more). However, currently you do still have to add the flag on the command line to node in order for it to look for `.only()` tests. If you add the command line flag and don't put `.only()` anywhere, nothing gets run. This is why we have added an additional VSCode task to allow us to run tests in 'only' mode.
Without this change a statement like `expect(result.succeeded).to.be.true` causes ESLint to raise a `no-unused-expressions` error.
Needed for our `base.request.js` module.
Aside from ESM syntax changes, the key change to highlight is we've had to rename `delete()` to `deleteRequest()`. There did seem to be an obvious workaround to `delete` being a keyword in JavaScript now we're using ESM syntax and imports/exports.
We also add the package-lock.js file now all our dependencies are added.
We demonstrate using Nock (nothing has changed) and how we can stub out GlobalNotifier, something we commonly do in our existing tests. We also demonstrate how we assert against the mock that it received the calls we expect.
Not added here, but we have tried to add the first `describe('because request timed out', () => {` test. However, we are currently stumped trying to find an alternate to `Sinon.replace()`.
What we can confirm is unlike Lab, node-test has no timeout on each individual test. You can specify one either at test or at global level. But if you don't then there is no timeout.
So, we can get the test to work if we tell Nock to delay for 6000ms (our request timeout is 5000ms). But obviously we don't want to really delay our test suite for 6 seconds. Hence we are currently blocked progressing these tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
base.request.jsmodule in water-abstraction-system is a great candidate for rewriting its tests using the node-test runner.In the module itself, we can bring in Got using a standard import. In the tests, we bring in Nock for the first time and confirm it works as before.
We also deal with mocking our
GlobalNotifierfor the first time (we've not even added it yet!) and make assertions against it.The key things we're hoping to get to the bottom of, though, is
Sinon.replace()Plus, while working on a new test, we give the updated
--test-onlyfunctionality a run for its money!