feat: add tests for shouldEnableProxy #3846
Conversation
Codecov Report
@@ Coverage Diff @@
## main #3846 +/- ##
==========================================
+ Coverage 62.60% 63.40% +0.80%
==========================================
Files 36 36
Lines 1872 1872
Branches 379 379
==========================================
+ Hits 1172 1187 +15
+ Misses 595 582 -13
+ Partials 105 103 -2
Continue to review full report at Codecov.
|
jsjoeio
force-pushed
the
jsjoeio-test-should-enable-proxy
branch
from
48ce0a7 to
67e9eca
Compare
jsjoeio
changed the title
test/unit/node/proxy_agent.test.ts
Outdated
| * | ||
| * Returns a function to cleanup the env variable. | ||
| */ | ||
| function setEnv(name: string, value: string) { |
There was a problem hiding this comment.
Nit: given this is a specific test helper, it would be nice if we could move the cleanup behavior into the beforeEach so that it may only have to be declared once. This is a bit like a React hook, and allows us to target a specific key, rather than dereferencing the entire process.env object.
// test_helpers.ts
function useEnv(key: string) {
const initialValue = process.env[key]
const setValue = (nextValue: string | undefined) => process.env[key] = nextValue
const resetValue = () => process.env[key] => initialValue
return [setValue, resetValue]
}
// test.ts
const [setHTTPProxy, resetHTTPproxy] = useEnv(“HTTP_PROXY”)
const [setNoProxy, resetNoProxy] = useEnv(“NO_PROXY”)
{
beforeEach() {
jest.resetModule()
resetHTTPproxy()
resetNoProxy()
}There was a problem hiding this comment.
Oh waaaay better than my solution. The original cleanup() I wrote was partially inspired by previous experience cleaning up useEffect functions. I really like your implementation. Thanks for writing that out! I'll make these changes.
|
🙌 Team work! |
This PR adds tests for
shouldEnableProxy()which is used inproxy_agent.ts.Related: