Opened 2 months ago

Closed 6 weeks ago

#27913 closed enhancement (fixed)

Add test-stem to travis-ci if possible

Reported by: nickm Owned by:
Priority: High Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: ci travis
Cc: teor Actual Points:
Parent ID: Points:
Reviewer: catalyst Sponsor:

Description


Child Tickets

Change History (17)

comment:1 Changed 2 months ago by rl1987

Status: newneeds_review

comment:3 Changed 2 months ago by nickm

Cc: teor added

adding teor to cc: they had ideas about how to do this

comment:4 Changed 2 months ago by catalyst

Reviewer: catalyst

comment:5 in reply to:  1 Changed 2 months ago by catalyst

Status: needs_reviewneeds_revision

Replying to rl1987:

https://github.com/torproject/tor/pull/387

Thanks! Mostly looks good. I left some minor comments on the pull request.

comment:6 Changed 2 months ago by rl1987

Status: needs_revisionneeds_review

comment:7 in reply to:  6 Changed 8 weeks ago by catalyst

Status: needs_reviewneeds_revision

Replying to rl1987:

Revised and rebased patch: https://github.com/torproject/tor/pull/399

Thanks, looks good! I did have one more small change that I wrote in a comment on the pull request. Sorry for not noticing the job speed thing earlier.

comment:8 Changed 8 weeks ago by rl1987

Status: needs_revisionneeds_review

comment:9 Changed 8 weeks ago by teor

Status: needs_reviewneeds_revision

I added a review to the pull request.

It's important that we log the stem version.
All of the rest of my comments are just stylistic.

comment:10 Changed 8 weeks ago by atagar

For what it's worth Stem's test run notes versioning information at its start...

======================================================================
                             INITIALISING                             
======================================================================


  stem version...                                      1.7.0
  tor version...                                       0.3.5.2-alpha-dev
  python version...                                    2.7.13
  operating system version...                          Linux (debian 9.5)
  cryptography version...                              missing
  pynacl version...                                    missing
  mock version...                                      2.0.0
  pyflakes version...                                  1.3.0
  pycodestyle version...                               missing
  checking for orphaned .pyc files...                  done (0.0s)
  checking for unused tests...                         done (0.0s)
  importing test modules...                            done (0.2s)
  running pyflakes...                                  running
  running pycodestyle...                               unavailable

======================================================================
                              UNIT TESTS                              
======================================================================

  util.enum...                                         success (0.00s)
  util.connection...                                   success (0.08s)
... etc...

Or did you mean the git commit id? If so then I'm kinda tempted to add that to the above output.

comment:11 Changed 8 weeks ago by teor

It's important that we have the stem version and git commit id, even if stem doesn't launch. (Or particularly if stem doesn't launch.)

Edit: italics

Printing the version is a standard thing we do for all our dependencies in Travis at the install stage.

Last edited 8 weeks ago by teor (previous) (diff)

comment:12 Changed 7 weeks ago by rl1987

Status: needs_revisionneeds_review

comment:13 Changed 7 weeks ago by teor

catalyst asked on IRC:

do you think the stem version check is good enough, or do you want it to be more robust against stem being broken?

I think it's ok to fail the tor build if python can't import stem.

But we'll need to be careful using stem master. If a bad commit is pushed to stem master, all our tor builds will break.

Usually, projects test against latest stable release of their dependencies. But major stem releases are a year apart, which means that we won't be testing the latest tor features. (And we really need to test the latest features.)

So if we're going to test tor against stem master, let's make sure that stem also tests against tor master (or nightly), so we catch broken builds as soon as possible.

I opened #28170 for this change to stem, I have to do the same thing for chutney in #27912, so I'll just copy the config across.

comment:14 Changed 7 weeks ago by catalyst

Status: needs_reviewmerge_ready

Summarizing some IRC discussion, it seems reasonable to just go ahead and try using stem master for running make test-stem, and disable it (or add it to allowed_failures) if it becomes too unreliable.

comment:15 Changed 7 weeks ago by teor

On the stem side, I'll try to do #28170 this week, but it might not be ready until late next week.

comment:16 Changed 6 weeks ago by nickm

Okay, I've rebased this onto 0.3.5 and made a fresh PR as https://github.com/torproject/tor/pull/454 .

(Since 0.3.5 is our LTS it makes sense for us to do this kind of thing there.)

comment:17 Changed 6 weeks ago by nickm

Resolution: fixed
Status: merge_readyclosed

it passed: merged to 0.3.5 and forward!

Note: See TracTickets for help on using tickets.