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 tests with race flag using checkflags script #1024

Merged
merged 5 commits into from
Apr 7, 2015

Conversation

chriscool
Copy link
Contributor

This is an alternative implementation of PR #1013 (Run tests with race flag) using a "checkflags" script.

As for PR #1013 the goal is to make it easier to find race conditions like issue #1012.

This script can be used in a Makefile to detect flag changes
and to save the new flags in a file.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
This makes it possible to build binaries with
different flags.

The content of the GOFLAGS variable is stored
in a IPFS-BUILD-OPTIONS file, so that if GOFLAGS
changes a rebuild of the binaries with the new
flags is forced.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
This builds go binaries using the -race flag
and then runs all the tests.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
...and remove bin/* stuff from the .gitignore
as /test/bin is already in the root .gitignore.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
The GOFLAGS variable makes it possible to run all sharness
tests with go binaries built with some special flags.

The "race" target makes it easy run the sharness tests
with go binaries built with the -race flag.

License: MIT
Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
@jbenet jbenet added the status/in-progress In progress label Apr 6, 2015
@jbenet
Copy link
Member

jbenet commented Apr 6, 2015

I think this LGTM, but can @whyrusleeping and @cryptix to take a look too.

also @chriscool did you see comment about building to another dir? say have parallel bins at both:

bin/ipfs
bin-race/ipfs

?

dont have to go this route (may be more annoying) but figured it was worth considering.

@chriscool
Copy link
Contributor Author

@jbenet yeah I saw your comment on PR #1013 and responded there.

About different names for directories, yeah we could do that, but if we have more flags and mix them I don't think it would scale well.

@jbenet
Copy link
Member

jbenet commented Apr 6, 2015

responded there.

ah missed that thanks

About different names for directories, yeah we could do that, but if we have more flags and mix them I don't think it would scale well.

yeah, it wont :) let's go with your approach.

@chriscool
Copy link
Contributor Author

@jbenet ok let me know if you want something before merging this or PR #1013.

@jbenet
Copy link
Member

jbenet commented Apr 7, 2015

I think this works great, thanks @chriscool

jbenet added a commit that referenced this pull request Apr 7, 2015
Run tests with race flag using checkflags script
@jbenet jbenet merged commit f2aba08 into master Apr 7, 2015
@whyrusleeping whyrusleeping removed the status/in-progress In progress label Apr 7, 2015
@jbenet jbenet deleted the run_tests_with_race_flag2 branch April 7, 2015 11:18
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.

3 participants