Fixes for the test to run on windows #71
Conversation
|
@promiseofcake can you also review this one ? it's quite straightforward and make the tests a bit more cross platform. (and my life as windows user a bit easier ;)) |
client/client_test.go
Outdated
| @@ -210,7 +210,11 @@ func TestUpdateFeatures(t *testing.T) { | |||
| }`) | |||
|
|
|||
| cfg := config.DefaultConfig() | |||
| cfg.Git.RepoPath = "/tmp" | |||
| dir, err := ioutil.TempDir("", "example") | |||
There was a problem hiding this comment.
May we generalize these into a shared function, something like this:
func TempDir(t *testing.T) string {
dir, err := ioutil.TempDir(os.TempDir(), "dcdr")
if err != nil {
t.Fatalf("ioutil.TempDir error: %s", err.Error())
}
return dir
}
We can also use t.Fatalf to allow the test-suite to handle the error gracefully. Also we should prefix the directory with a related to this project: dcdr or similar.
Then from each test case we can call the parent function and not need to handle the errors:
dir := TempDir(t)
defer os.RemoveAll(dir)
cfg.Git.RepoPath = dir
client/client_test.go
Outdated
| c, _ := New(cfg) | ||
| c.UpdateFeatures(badUpdate) | ||
| assert.EqualValues(t, models.EmptyFeatureMap(), c.FeatureMap(), "Assert bad payload returns empty feature map") | ||
| } | ||
|
|
||
| func TestWatch(t *testing.T) { | ||
| p := "/tmp/decider.json" | ||
| tmpfile, err := ioutil.TempFile("", "example") |
There was a problem hiding this comment.
Let's name this decider.json to be in line with what we are expecting. I know that the library assumes "" to be the default temporary directory, but maybe we can be more explicit with: os.TempDir().
It seems like it's up to us to clean up this file as well.
| @@ -4,6 +4,8 @@ import ( | |||
| "os/user" | |||
| "testing" | |||
|
|
|||
There was a problem hiding this comment.
May we lint these imports? I know this was an error before you change but since you're here.
There was a problem hiding this comment.
I've fixed the other issues, but I'm not sure what exactly you mean like this imports... care to elabrate ?
There was a problem hiding this comment.
Sure, it's a nitpick but the imports, the declarations for dependencies for this file, are not ordered properly:
import (
"encoding/json"
"io/ioutil"
"log"
"os"
"testing"
"github.com/stretchr/testify/assert"
"github.com/vsco/dcdr/config"
"github.com/vsco/dcdr/models"
)
Unfortunately goimports does not catch this lint error due to the spaces between the groups yet, but work is being done here: golang/go#20818
This is not necessary to merge this PR.
There was a problem hiding this comment.
ordering of imports, got it (i'll try to noticed it next time around)
config/config_test.go
Outdated
| @@ -41,7 +43,12 @@ func TestDefaultConfig(t *testing.T) { | |||
| } | |||
|
|
|||
| func TestEnvOverride(t *testing.T) { | |||
| os.Setenv(envConfigDirOverride, "/tmp/dcdr") | |||
| dir, err := ioutil.TempDir("", "example") | |||
There was a problem hiding this comment.
Same, as above let's use a more relatable name.
|
seem like this flanky test failed me again... |
|
@promiseofcake can you rerun it ? (insted of me pushing something again) |
|
@promiseofcake I do love race conditions 😄 |
|
@fruch, thanks for the contribution! Ah yes, we should fix that 😄. |
Utilize OS specific temp directories/files rather than relying on
/tmp