Support a second API func to control environments source

[argos83]

I'd like to propose that a second API function is exported, that allows control over the source of the environment. Mostly for testeability (but I guess it could serve other purposes).

I.e. From:

Process(prefix string, spec interface{}) error {
    ... your implementation ...
}

To:

type LookupEnvFunc = func(key string) (string, bool)

//Probably a better name
ProcessEnv(prefix string, spec interface, lookupEnv LookupEnvFunc) error {
    ... your implementation using the given lookupEnv function...
}

Process(prefix string, spec interface{}) error {
    lookupEnv := os.lookupEnv
    if golang version is something.something {
      lookupEnv := syscall.Getenv
    }

    return ProcessEnv(prefix, spec, lookupEnv)
}

Reason for this proposal: Currently, the only way to test my env parsing config is via the real os environment, this is an anti-pattern since it creates interdependency between tests and may result in other unexpected consequences in different places that rely on the environment. For instance, in your tests you need to call os.Clearenv() every the time to workaround this interdependency.

The environment is an external global dependency the same as the clock, fileysystem, stdin/stdout, etc. So testeability improves significantly if all these can be mocked.

By having control of the env dependency, a mock can be injected instead. A common testing pattern would then be:

production_code.go

// Almost all my unit/component tests will call this function with a mock
func myAppEntryPoint(lookupEnv LookupEnvFunc) err {
    ... all my app logic ...
    ...
    envconfig.ProcessEnv("my-app", config, lookupEnv)
    ...
    .. etc ..

}

// One or two integration tests will call this to veryfy this thin integration layer, using the real env.
func main() {
    if err := myAppEntryPoint(os.LookupEnv); err != nil {
       log.Fatalf("failed to start app  %v", err)
    }
}

production_code_tests.go

// pseudo example using testify

func (suite *TestSuite) SetupTest() {
   suite.mockEnv = mock.NewEnv()
}

func (suite *TestSuite) whenMyAppIsCalled() error {
   return myAppEntryPoint(mockEnv.LookupEnv)
}

...
func (suite *TestSuite) TestAppFailsIfPortIsSetWithAnInvalidValue() {
    suite.mockEnv.givenEnvironmentVariable("PORT", "not-a-number")
    err := suite.whenMyAppIsCalled()
    assert.EqualError(t, err, "assigning PORT to Port: converting 'not-a-number' to type int")
}

func (suite *TestSuite) TestAppUsesPort8080ByDefault() {
    suite.mockEnv.givenVariableUnset("PORT")
    suite.whenMyAppIsCalled()
    // assert something to verify port is 8080
    // could assert on the config object if unit test, or some side effect for a component test.
}
...

Notice this change would be a backwards compatible (the Process function would behave the same way).

Furthermore, this test pattern could be used in your own tests. I noticed you use os.LookupEnv or syscall.GetEnv depending on the golang version. So all parsing logic could be tested with a mock (via using ProcessEnv) regardless of the golang version. But you have a few cases to test Process directly to make sure that thin integration bit still works.

(Or maybe you won't longer need to make that distinction, if I understand correctly the issue was some caching being done by go in LookupEnv, so you had to bypass that using syscall.GetEnv. The environment is not supposed to mutate in a process, so I guess the golang people relied on this assumption and implemented a cache, which caused you trouble because of the tests interdependency)

If you agree with this, I might be able to find some time and raise a pull request, but I wanted to discuss it with you first.

Let me know what you think.