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

Run all Go tests with flag -race #7783

Merged
merged 3 commits into from
Jul 7, 2020
Merged

Run all Go tests with flag -race #7783

merged 3 commits into from
Jul 7, 2020

Conversation

programmer04
Copy link
Contributor

@programmer04 programmer04 commented Jul 3, 2020

Required for all PRs:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Running

go test -race -short ./...

on current master finishes without any errors, so it's a great opportunity to add -race flag to all invocation of tests and closes #7729 🎉

In the Makefile for all invocation of tests if run not with GOARCH=386 flag (Go race detector works only on 64 bit architectures) flag -race is added.

@ssoroka
Copy link
Contributor

ssoroka commented Jul 3, 2020

hmm. apparently not supported on 386 architectures.

@programmer04
Copy link
Contributor Author

@ssoroka I'm modifying makefile to not run them with the flag -race for GOARCH=386

@danielnelson danielnelson added this to the 1.16.0 milestone Jul 6, 2020
@danielnelson
Copy link
Contributor

Let's make this a new phony target test-race and run it via Circle and Appveyor.

@ssoroka
Copy link
Contributor

ssoroka commented Jul 6, 2020

looks like there's a race affecting windows only.
plugins/inputs/ping/ping_windows.go
remove this at line 16:

	if p.Count < 1 {
		p.Count = 1
	}

It's redundant as it's checked on init, and causing the race condition.

@danielnelson
Copy link
Contributor

Another option instead of the phony target is to do nothing in the Makefile, but set GOFLAGS="$GOFLAGS -race" in the CI configuration. I'm not 100% sure if this flag is honored though, would need tested.

@programmer04
Copy link
Contributor Author

programmer04 commented Jul 6, 2020

I didn't have time and Win setup to check it in detail. On other systems, it works fine 🙂
@ssoroka I applied your suggestion and tests pass on Windows too. So on every platform except 386 all tests are run with -race enabled. Thanks for guidance 🙂

@ssoroka ssoroka merged commit 876fc5b into influxdata:master Jul 7, 2020
@programmer04 programmer04 deleted the enable-data-race-detection branch July 7, 2020 19:30
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running tests with Go built-in data race detector enabled exposes a plenty of data races
3 participants