search

graph-gophers / dataloader

Implementation of Facebook's DataLoader in Golang
MIT License
1.2k stars 75 forks source link

Fix install command in travis config #73

Closed mjq closed 3 years ago

mjq commented 3 years ago

I noticed that my build was failing in #72. I think it's due to the command used in the install step.

From the travis logs:

$ go mod install
go mod install: unknown command

This was probably meant to be go mod download. Hopefully this fixes the build! (Edit: Looks like it did!)

mjq commented 3 years ago

Looks like a flaky test in CI maybe? My first push (c3e8926) had a green CI (passed 2/2 checks), my second (6b7905c, identical to the first but committed with the correct email address) failed 1/2 checks.

Edit: I can reproduce this failure, but only rarely - on my laptop, the test failed only 3 out of 10000 runs.

$ go test -v -race -coverprofile=coverage.txt -covermode=atomic -count 10000 | grep FAIL
--- FAIL: TestLoader (0.00s)
    --- FAIL: TestLoader/allows_clearAll_values_in_cache (0.02s)
--- FAIL: TestLoader (0.00s)
    --- FAIL: TestLoader/allows_clearAll_values_in_cache (0.02s)
--- FAIL: TestLoader (0.00s)
    --- FAIL: TestLoader/allows_clearAll_values_in_cache (0.02s)
FAIL
FAIL    github.com/graph-gophers/dataloader/v6  390.973s
tonyghita commented 3 years ago

@mjq thanks for the pull request. Looks like we got lucky catching the test flake. I'm having a hard time reproducing it but that seems like an issue we'll want to investigate separately.

I haven't been able to reproduce the failure locally yet but I've only been running -count=1000.

I'm not super familiar with TravisCI but I think the default installation mechanism go get ./... works fine so long as we're in module-mode. I've opened a similar PR but with a different approach in #75.

tonyghita commented 3 years ago

I opened a new issue to track the test flake https://github.com/graph-gophers/dataloader/issues/76 and merged in #75. Thank you very much for your contribution!

mjq commented 3 years ago

Awesome! Thanks for following up on those :+1:

tonyghita commented 3 years ago

@mjq after a bit of digging I've opened #77 to resolve the test flake issue. I'm unable to reproduce the flakiness on my local machine with those changes.