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

#27913: Try running test-stem on Travis (2) #399

Closed
wants to merge 9 commits into from

Conversation

Labels
None yet
Projects
None yet
5 participants
@rl1987
Copy link
Contributor

@rl1987 rl1987 commented Oct 12, 2018

https://trac.torproject.org/projects/tor/ticket/27913

@rl1987 rl1987 changed the title #27913: Try running test-stem on travis #27913: Try running test-stem on Travis (2) Oct 12, 2018
@coveralls
Copy link

@coveralls coveralls commented Oct 12, 2018

Coverage Status

Coverage increased (+0.06%) to 62.015% when pulling 54dfc45 on rl1987:travis_test_stem_2 into 391756f on torproject:master.

.travis.yml Outdated
@@ -200,6 +202,7 @@ script:
- ./configure $CONFIGURE_FLAGS
## We run `make check` because that's what https://jenkins.torproject.org does.
- if [[ "$DISTCHECK" == "" ]]; then make check; fi
Copy link
Contributor

@tlyu tlyu Oct 15, 2018

I just noticed this job takes significantly longer to run, so maybe we could skip the make check when we're doing make test-stem? We'd probably want to do something like make all && make test-stem though to avoid parallelization problems.

Copy link

@onioncount onioncount Oct 15, 2018

+1

I wish we could fix the parallelisation problems, but that's a separate issue that deserves a separate ticket. (And they're so hard to debug.)

I suggest make src/app/tor, unless test-stem uses some of the other build products.

(Oops, this is teor, I am logged into the wrong account.)

Copy link
Contributor Author

@rl1987 rl1987 Oct 16, 2018

Did these things and build takes less time now.

rl1987 added 2 commits Oct 16, 2018
When running `make test-stem` on Travis, we should refrain from
also running `make check`. Furthermore, let's limit compilation
to src/app/tor target.
.travis.yml Outdated
@@ -199,7 +201,8 @@ script:
- echo "Configure flags are $CONFIGURE_FLAGS"
- ./configure $CONFIGURE_FLAGS
## We run `make check` because that's what https://jenkins.torproject.org does.
- if [[ "$DISTCHECK" == "" ]]; then make check; fi
- if [[ "$DISTCHECK" == "" && "$TEST_STEM" == "" ]]; then make check; fi
- if [[ "$TEST_STEM" != "" ]]; then git clone --depth 1 https://github.com/torproject/stem.git ; export STEM_SOURCE_DIR=`pwd`/stem; make src/app/tor test-stem; fi
Copy link
Contributor

@teor2345 teor2345 Oct 16, 2018

Technically, git clone stem belongs in the install section.
We also do our environmental variable setup in install.
Please also log the stem version. I think git clone already logs the stem commit.

It seems like distcheck and test-stem are incompatible, because distcheck deletes src/app/tor.
If both options are specified, should we skip stem and log a message?

Nitpick: I'd like to see test stem added as the last item in both sections, but it doesn't really matter.

Copy link
Contributor Author

@rl1987 rl1987 Oct 20, 2018

These are addressed in further commits now. Opened a new pull request with rebased branch and cleaned up git history: #423

@rl1987 rl1987 closed this Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment