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 #1013

Closed
wants to merge 4 commits into from
Closed

Run tests with race flag #1013

wants to merge 4 commits into from

Conversation

chriscool
Copy link
Contributor

This should make it easier to find race conditions like issue #1012.

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>
@whyrusleeping whyrusleeping added the status/in-progress In progress label Apr 4, 2015
if test x"$$VARS" != x"`cat $@ 2>/dev/null`" ; then \
echo "*** new Go flags ***"; \
echo "$$VARS" >$@; \
fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems very magic-y to me. wonder if maybe the right thing to do is to have both:

bin/program
bin/program-race

that way we could even have concurrent tests going

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied what is done in the Git Makefile for these kind of things.
It is just checking if the content of the GOFLAGS variable is different from what is in the IPFS-BUILD-OPTIONS file, and if it is different, it puts the content of the GOFLAGS variable into the file.

We could have something like:

bin/program-normal
bin/program-race

and then a bin/program symlink that points to one or the other of the above binaries. But I am not sure it would be simpler and this way we couldn't have concurent tests going on.

To have concurent tests going on, we would need to modify the test scripts before launching them with something like sed 's/ipfs/ipfs-race/g' but it could be tricky to get right too.

Or maybe I am missing something?

@chriscool
Copy link
Contributor Author

Note that I used this patch series to check if there are race conditions in all the sharness tests, and I found some only in t0090-get.sh (see issue #1012).

@chriscool
Copy link
Contributor Author

Closing this as the alternative PR #1024 is merged.

@chriscool chriscool closed this Apr 7, 2015
@whyrusleeping whyrusleeping removed the status/in-progress In progress label Apr 7, 2015
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