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:
Add testCaseMatrix to the tests/utils.js module and generalize some of the functions in that module.
The test case matrix is an array of objects that are used to inject the test case-specific parameters: whether to start a secure server or not, whether to start a secure proxy or not, the agent class to test, and a text description.
Merge http-http.test.js, https-http.test.js, http-https.test.js and https-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 in https-http and http-https
Test Host Header was missing in http-https, https-http and https-https
Merge the four tests into a single parameterized test in each of got.js, needle.js, node-fetch.js and simple-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.
Now that #72 landed, I'll need to rebase this PR and fix the merge conflicts. Before I do that, could you please at least let me know if you're interested in this change?
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.