test: refactored test suite to use mocks #636
Merged
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
added 20 commits
July 24, 2020 13:56
I.e. where test cases call execCliAsync() or require('./index')
This drops the following to tests: - throws error when not specifying a release type - exits with error code if "scripts" is not an object - exits with error code if "skip" is not an object - handles commit messages longer than 80 characters - includes merge commits These were not testing standard-version, but yargs and conventional-changelog.
This temporarily makes the test suite output rather verbose.
Using a require() with a user-controllable path can be dangerous, as it allows for directory traversal with an updater.type like '../../../../somewhere/else', and it makes mocking the filesystem rather difficult.
Sets core.autocrlf false in git config, for Windows environments
This drops mock-git as a dependency
|
Fixed the remaining Windows test issues. Where previously the CI timings for the test suite took were 6min for the Ubuntu tests & a bit under 9min on Windows, it now takes 1.5 min & a bit under 4 min, respectively. I also added an |
bcoe
approved these changes
Sep 7, 2020
bcoe
reviewed
Sep 7, 2020
| @@ -1,13 +1,17 @@ | |||
| const path = require('path') | |||
| const JSON_BUMP_FILES = require('../../defaults').bumpFiles | |||
| const updatersByType = { | |||
There was a problem hiding this comment.
I like this refactor but it should probably be in a separate PR?
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 test suite was really slow, to the extent that it made working on the library difficult. So I mostly fixed that here. Where previously it could take multiple minutes for it to complete, it's now at about 15 seconds total on my machine. And if you skip the commit & tag tests (now in
test/git.spec.js), it's about 2 seconds. The coverage reporting does add a bit to that, of course.Mostly the time savings come from not spawning wrapper processes around everything and mocking external dependencies and the filesystem whenever possible. A few tests are also dropped, as they were only testing external dependencies, rather than this library's own code.
The only non-test-code change is to
lib/updaters/index.js, where the type-specific updaters are now loaded at the start, rather than on demand; this made mocking the filesystem around them significantly easier.Code coverage does not change significantly during this change. Tests are split into three files, and moved to the
test/directory.