Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests should run cleanly when passed the -race option #35

Closed
jdharms opened this issue Sep 13, 2019 · 4 comments
Closed

Tests should run cleanly when passed the -race option #35

jdharms opened this issue Sep 13, 2019 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers ireland

Comments

@jdharms
Copy link
Contributor

jdharms commented Sep 13, 2019

To ensure our tests (and to some extent production code) are reliable, we should be running unit tests with the -race argument in the Makefile. Currently, the tests fail when run in this mode.

Failing tests:

  • TestRegisterWithPingCallback() in internal/pkg/consul/client_test.go
  • TestIsServiceAvailableNotHealthy() in internal/pkg/consul/client_test.go
  • TestConfigurationValueExists() in internal/pkg/consul/client_test.go
  • TestPutConfiguration() in internal/pkg/consul/client_test.go
  • TestWatchForChanges() in internal/pkg/consul/client_test.go

This test should succeed when passed -race, and then we should add -race to the test target in our Makefile.

@michaelestrin michaelestrin added enhancement New feature or request geneva labels Dec 17, 2019
@brandonforster brandonforster added hanoi Hanoi release and removed geneva labels Feb 27, 2020
@brandonforster
Copy link
Contributor

Per discussion on DevOps WG call on 27 February, this has been rescoped from Geneva to Hanoi so that DevOps can address some underlying build concerns.

@brandonforster brandonforster removed their assignment Mar 25, 2020
@brandonforster brandonforster added the good first issue Good for newcomers label Mar 25, 2020
@tsconn23
Copy link
Member

After some local testing, it appears the tests called out in the original issue are now passing. I will create a PR that incorporates the race flag into the Makefile so that this condition is always on and then see how the CI/CD pipeline handles it.

@tsconn23
Copy link
Member

tsconn23 commented Aug 18, 2020

See comment on PR #48 above. While the initial tests called out in the issue are no longer a problem, enabling the -race flag for use in JJB's Alpine-based containers cannot be done until the project adopts Go 1.15

@jpwhitemn jpwhitemn added ireland and removed hanoi Hanoi release labels Oct 22, 2020
@jpwhitemn
Copy link
Member

DevOps addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers ireland
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants