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.
This looks like a big PR but it can be summarized simply as factoring out all the repeated code in tests that cover the exact same flow but with one of the four combinations (http over http, http over https, https over http, https over https). It implements the DRY principle in the tests.
The PR changes are as follows:
testCaseMatrix
to thetests/utils.js
module and generalize some of the functions in that module.http-http.test.js
,https-http.test.js
,http-https.test.js
andhttps-https.test.js
into `common-proxy.test.js. In addition to reducing line count by ~4x, it adds 5 tests that was missing:Username and password should not be encoded
was missing inhttps-http
andhttp-https
Test Host Header
was missing inhttp-https
,https-http
andhttps-https
got.js
,needle.js
,node-fetch.js
andsimple-get.js
I recommend taking this PR to avoid writing 4x the number of tests every time a feature is added, and guarantee that test coverage is the same for all variations. You can still implement tests specific to individual combinations but currently none is needed.
This is the PR I mentioned in this Issue comment.
Note: this PR introduces a merge conflict with #72. Whichever lands first will require a manual merge before the other can land.