fix: Support environment variable system PATH test on Windows machines

[JayCanuck]

I noticed on my Windows machine it used Path variable key in process.env. While in normal cases that would resolve process.env.PATH case-insensitive correctly, the act of copying system vars within dotenv resulted in the case-sesitive Path entry, which would fail the test. Added a workaround to ensure uppercase PATH is set on Windows machines appropriately.

Hardly a necesary PR to take, but wanted to offer it if interested.

[mrsteele]

@JayCanuck thats actually pretty interesting... I'm not sure how to move forward...

[JayCanuck]

Yea, definitely an optional PR. Travis isn't using Windows (and I suspect most people running the tests aren't either). Only figured worth making a PR for consideration given the git-hooks that run the tests pre-push.

Could alternatively modify the tests to set some value to process.env.something then run the envTest against that property, rather than relying on pre-existing env vars for the test. Might be cleaner.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@b0946db). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #132   +/-   ##
=========================================
  Coverage          ?   98.03%           
=========================================
  Files             ?        1           
  Lines             ?       51           
  Branches          ?        0           
=========================================
  Hits              ?       50           
  Misses            ?        1           
  Partials          ?        0

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0946db...9c8b69c. Read the comment docs.

[tysonmatanich]

If the tests worked out of the box on Windows it would make it a lot easier for Windows users to contribute. I had to make edits to the tests to get it working locally (not fun) and then had to revert those changes before committing (risks mistakes). The test in question should just use a different environment variable that will work cross platform.

[mrsteele]

Sorry about this, but its resolved in the latest PR. Closing this one in favor of the other one. Definitely a good observation.

[tysonmatanich]

@mrsteele which pull request fixes it out of curiosity?

[mrsteele]

#266

[tysonmatanich]

@mrsteele Tests still fail on my Windows 10 machine due to use of PATH instead of Path in the following files:

• test/index.test.js
• test/envs/.systemvars
• test/envs/.systemvars.example

Any reason to not use a different environment variable that doesn't vary like this? Seems like it would also make the test cleaner so it doesnt need the \ and / logic.

[mrsteele]

@tysonmatanich Is that the norm with Windows 10? My env is PATH and I'm on Windows 10 now and tests are passing.

[mrsteele]

I just read this on the node docs:

On Windows operating systems, environment variables are case-insensitive.

https://nodejs.org/api/process.html

Do you have a recommendation of any other environment variable? Or maybe we can set up a variable that we know should exist on the system? The latter might be the safest:

process.env.FORCED = 'I made this here'
const FORCED = envTest({ safe: envSystemvarsExample, systemvars: true })['process.env.FORCED']

Thoughts?

[tysonmatanich]

I never changed the path variable on my system, not sure why ours are different. Perhaps a newish fresh install is different than an older system updated to the same version (just a guess).

I think the method of setting an environment variable would be the safest and most reliable.

[mrsteele]

@tysonmatanich wanna take a stab at a PR?

I can do it, but probably wont be for a little bit.