Opened 4 years ago

Closed 4 years ago

#14804 closed defect (fixed)

Confusing arguments to run_tests.py

Reported by: Sebastian Owned by: atagar
Priority: Medium Milestone:
Component: Core Tor/Stem Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

There seems to be some confusion going on about the overloading of -t to mean --test and --target:

./run_tests.py --integ --test test.integ.control.controller ONLINE <-- this runs without complaint (without enabling the online tests), tho it should be an error because of the dangling ONLINE

./run_tests.py --integ --test test.integ.control.controller -t ONLINE <-- this runs without an error, but runs no tests at all. It reports "TESTING PASSED (0seconds)" which is unhelpful, because it doesn't say that it ran 0 tests

./run_tests.py --integ --test test.integ.control.controller --target ONLINE behaves exactly the same like the above.

Child Tickets

Change History (3)

comment:1 Changed 4 years ago by atagar

Oops! You're right, we treat '-t' as being shorthand for both and due to ordering it becomes in effect '--targets'...

elif opt in ('-t', '--targets'):
  ... parse...
elif opt in ('-t', '--test'):
  ... more parsing...

Easy to fix. Thanks for the catch!

comment:2 Changed 4 years ago by atagar

On reflection the '-t' alias isn't related at all to what you're talking about. Anyway, fixed.

You're actually highlighting a couple more interesting usability issue. Those are now fixed too...

Stem's integration test targets are split into two camps...

  • Run targets, which all start with 'RUN_'. Think of these as a torrc. Each one you provide means we do a run of our integration tests, so '--target RUN_COOKIE,RUN_PASSWORD' would run our tests twice. You can use 'RUN_ALL' to do all of them.
  • Attribute targets like 'ONLINE' and 'CHROOT'. These change characteristics of the test runs but aren't a separate run of their own.

When you provided '--target ONLINE' it replaced our default run target, which is why the tests became a no-op. If only attribute targets are provided we now keep the default so this should behave and you expected.

Thanks for the catches!

comment:3 Changed 4 years ago by atagar

Resolution: fixed
Status: newclosed

Oops, looks like I forgot to close this.

Note: See TracTickets for help on using tickets.