Tests on windows

[zenflow]

Resolves #241 (sorry for taking so long to get around to this)

Opening this PR early to test the AppVeyor configuration

[coveralls]

Coverage remained the same at 100.0% when pulling f9ec5ae on zenflow:tests-on-windows into 472db2e on motdotla:master.

[coveralls]

Coverage remained the same at 100.0% when pulling f9ec5ae on zenflow:tests-on-windows into 472db2e on motdotla:master.

[coveralls]

Coverage remained the same at 100.0% when pulling f9ec5ae on zenflow:tests-on-windows into 472db2e on motdotla:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 042b058 on zenflow:tests-on-windows into 472db2e on motdotla:master.

[zenflow]

Ok, for reference, here is the failing test output from issue #241 ...

> lab



  x..xx.x...............

Failed tests:

  1) config preload loads .env:

      Command failed: C:\Program Files\nodejs\node.exe -r ../config -e "console.log(process.env.BASIC)" dotenv_config_path=./test/.env
'C:\Program' is not recognized as an internal or external command,
operable program or batch file.


  'C:\Program' is not recognized as an internal or external command,
  operable program or batch file.
  
      at ChildProcess.exithandler (child_process.js:275:12)
      at emitTwo (events.js:126:13)
      at ChildProcess.emit (events.js:214:7)
      at maybeClose (internal/child_process.js:925:16)
      at Socket.stream.socket.on (internal/child_process.js:346:11)
      at emitOne (events.js:116:13)
      at Socket.emit (events.js:211:7)
      at Pipe._handle.close [as _onclose] (net.js:554:12)

  4) dotenv config reads path with encoding, parsing output to process.env:

      actual expected

      01

      expected 0 to equal 1

      at Assertion.fail (C:\Users\mbrunetti\Documents\dotenv\node_modules\should\cjs\should.js:205:17)
      at Assertion.value (C:\Users\mbrunetti\Documents\dotenv\node_modules\should\cjs\should.js:277:19)
      at C:\Users\mbrunetti\Documents\dotenv\test\main.js:56:34
      at Immediate.setImmediate [as _onImmediate] (C:\Users\mbrunetti\Documents\dotenv\node_modules\lab\lib\runner.js:647:36)
      at runCallback (timers.js:789:20)
      at tryOnImmediate (timers.js:751:5)
      at processImmediate [as _immediateCallback] (timers.js:722:5)

  5) dotenv config makes load a synonym of config:

      actual expected

      01

      expected 0 to equal 1

      at Assertion.fail (C:\Users\mbrunetti\Documents\dotenv\node_modules\should\cjs\should.js:205:17)
      at Assertion.value (C:\Users\mbrunetti\Documents\dotenv\node_modules\should\cjs\should.js:277:19)
      at C:\Users\mbrunetti\Documents\dotenv\test\main.js:64:34
      at Immediate.setImmediate [as _onImmediate] (C:\Users\mbrunetti\Documents\dotenv\node_modules\lab\lib\runner.js:647:36)
      at runCallback (timers.js:789:20)
      at tryOnImmediate (timers.js:751:5)
      at processImmediate [as _immediateCallback] (timers.js:722:5)

  7) dotenv config does not write over keys already in process.env if the key has a falsy value:

      Cannot read property 'should' of undefined

      at C:\Users\mbrunetti\Documents\dotenv\test\main.js:82:24
      at Immediate.setImmediate [as _onImmediate] (C:\Users\mbrunetti\Documents\dotenv\node_modules\lab\lib\runner.js:647:36)
      at runCallback (timers.js:789:20)
      at tryOnImmediate (timers.js:751:5)
      at processImmediate [as _immediateCallback] (timers.js:722:5)


4 of 22 tests failed
Test duration: 46 ms
The following leaks were detected:WebAssembly

npm ERR! Test failed.  See above for more details.

[zenflow]

So as it turns out, tests 4 and 5 are failing on Windows because they're failing everywhere, linux included... The lab -r lcov | coveralls command (used in the package's test script) does not give any console output, and returns an exit code of zero after it does it's job (reporting coverage) even if tests are not passing. Check out the Travis build output of 7811099 where I run tests with a normal human-readable output .. https://travis-ci.org/motdotla/dotenv/builds/346184038?utm_source=github_status&utm_medium=notification

[zenflow]

Ok

Test 1 is fixed by 1fcf13b, pretty simple. On Windows, node binary path is usually in C:\Program Files which has a space in the name.

Tests 4 and 5 are fixed by d25aee4. This is the test that was actually failing on linux too. Thankfully it was a mistake in the test, not in the source. But I did make a change to the source, so that the test would work as expected; I made a change that allows dotenv.parse to be stubbed.

Test 7 is fixed by 0a01850. This seems to indicate a (weird) portability issue with the nodejs platform itself (see zenflow/node-bug-windows-process-dot-env) but, going by the description of the test, it is not important that process.env.test equals '', just that it is not overwritten by dotenv.

[zenflow]

The last issue seems to be that (in all environments) lab fails due to it's "global variable leak detection" feature. After fixing all each failing test, we still get a non-zero exit code, and this message: The following leaks were detected:WebAssembly. This is because we are currently using an older version of lab which will fail on newer versions of node. See hapijs/lab#697 and hapijs/lab#581 (comment).

I updated lab to the latest with 51c239e, but now the tests won't work on older versions of node because they are unsupported (lab fails due to it's use of async functions).

That seems like a subtly big problem: our test runner does not support the range of nodejs versions that we do.

I can't see any immediate solution to that (except changing the test runner, and that seems out-of-scope for this PR) so for now I will revert 51c239e and simply disable the "global variable leak detection" feature of lab.

[zenflow]

Opening this PR early to test the AppVeyor configuration

This PR is now ready for review (and merging too I think)

/cc @motdotla @maxbeatty @jcblw

[maxbeatty]

Thanks for all of this! In #279, I was able to upgrade lab to avoid turning off leak detection and rework the parse stubbing to avoid changing the source. Patch release will go out soon 🙏

If you have the time, it would be worth following up on why Windows turns empty string assignments to process.env into undefined

[zenflow]

@maxbeatty Glad I could help. To be honest, a little disappointed that you didn't think this PR was worth reviewing/discussing and merging, and you rather copy the work into your own PR. But I'll remind myself that that's not really important 😄.

The nodejs bug is really beyond me; no chance I'll be able to figure out why. (It's so random in when it pops up, but thankfully it can be consistently reproduced) But I did file this bug report with a reproduction repository: nodejs/node#19048

[maxbeatty]

Please know your contribution is appreciated. Sometimes it's easier to pull in changes manually than exchange multiple rounds of reviews.

If dotenv's codebase can be cleaner and simpler after the v8.10.0 release, we would welcome more contributions.