Opened 4 months ago

Closed 2 months ago

#22943 closed enhancement (fixed)

Add Travis CI configuration so that Github forks receive CI coverage

Reported by: patrickod Owned by: atagar
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Normal Keywords: continuous-integration ci unit-testing testing new-developers travis best-practice
Cc: isis, cc@… Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Similarly to #22636 we should add a Travis CI configuration with a comprehensive test matrix to the project root so that developers working on their own Github forks receive easy access to working CI setups.

Child Tickets

Change History (9)

comment:1 Changed 4 months ago by atagar

Status: newneeds_information

Hi Patrick, Stem already has CI via Jenkins...

https://jenkins.torproject.org/job/stem-tor-ci/

They're broken at the moment but that's another story...

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

Folks indeed make GitHub forks to send me pull requests but I'm unsure why CI would be important to them. They pull from master, make their change, run tests, and send me a pull request. It's not a long-lived fork. If it is they'd certainly be welcome to add this to themselves.

Is there something I'm missing?

comment:2 in reply to:  1 Changed 4 months ago by isis

Status: needs_informationnew

Replying to atagar:

Hi Patrick, Stem already has CI via Jenkins...

https://jenkins.torproject.org/job/stem-tor-ci/

They're broken at the moment but that's another story...

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

Folks indeed make GitHub forks to send me pull requests but I'm unsure why CI would be important to them. They pull from master, make their change, run tests, and send me a pull request. It's not a long-lived fork. If it is they'd certainly be welcome to add this to themselves.

Is there something I'm missing?


In terms of the benefits to newcomers, I think of it more as a friendly way of saying "here's what I expect to pass if you fork my code and change it" and giving new developers a super easy way to prove to you (before you spend time on code review) that they met that expectation.

W.r.t. benefits to core maintainers/developers, I find it's really nice to not have to wait until something is in the master (or whichever maintenance branch) of the official main repo to know if something broke, or if it had a bad reaction with something else that was merged, etc. (This is maybe more beneficial to things like little-t tor which have a lot of people hacking in different directions all at once, but Stem contributors are also growing and it's getting a lot of love from different people.) Also for me, I feel like it makes the development workflow much faster, because instead of "make commit, run tests locally, wait, repeat" it's more like "make commit, push, repeat, (possibly get notified of breakage and make a fixup commit)". It also serves to assure me that it's not "just working on my machine" but that another machine out there is able to get the same test results. Also, it's somehow incredibly satisfying to watch e.g. https://travis-ci.org/isislovecruft/bridgedb/builds/ and see all the little icons turn from yellow to (usually, hopefully) green.

I can probably go on and on more about how great it is to have CI everywhere all the time, if you want more reasons. :)

comment:3 Changed 4 months ago by atagar

Thanks Isis, sounds good. We already have a tox stuff I don't use so if this has folks and isn't very invasive I'd be happy to take a patch.

comment:4 Changed 2 months ago by ewong

Here's my attempt at it:
https://github.com/ewongbb/stem/tree/add_travis

(it's only for stem right now..)

currently integration tests are busted.. as .travis.yml isn't in the tor release tarball..
but since I haven't gotten to the point of installing tor on my system, I
haven't successfully run any integration tests.

comment:5 Changed 2 months ago by ewong

Cc: cc@… added

comment:6 Changed 2 months ago by ewong

Here's a sample of the test:

https://travis-ci.org/ewongbb/stem

comment:7 Changed 2 months ago by ewong

Status: newneeds_revision

comment:8 Changed 2 months ago by ewong

Status: needs_revisionneeds_review

comment:9 Changed 2 months ago by atagar

Resolution: fixed
Status: needs_reviewclosed

Thanks! Merged.

Note: See TracTickets for help on using tickets.