Opened 3 months ago

Closed 3 months ago

#31554 closed enhancement (fixed)

Restrict "make test-stem" to tests that actually use tor

Reported by: teor Owned by: teor
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-ci, fast-fix, 041-backport, 040-backport, 035-backport, dgoulet-merge
Cc: gaba, atagar Actual Points: 0.2
Parent ID: Points: 0.1
Reviewer: nickm Sponsor: Sponsor31-can

Description

In #30694, we restricted the travis stem job to tests that actually use tor.

But we should lower that change to "make test-stem".

Gaba, this is sponsor 27 can, because it makes refactoring easier to test.

Child Tickets

Change History (11)

comment:1 Changed 3 months ago by teor

Actual Points: → 0.2
Status: assigned → needs_review

See my pull request:
0.3.5: ​https://github.com/torproject/tor/pull/1271

This change merges forward cleanly to master.

The test branches are called ticket31554_* in my repository:
​https://github.com/teor2345/tor/branches

comment:2 Changed 3 months ago by nickm

This looks reasonable to me, but I wonder if it will be stable long-term. If stem adds a new group of tests that we want to cover, will it necessarily be under control.controller, control.base_controller, or process ?

(That's not a blocking problem for this ticket, but it does suggest to me that we should ask stem to incorporate an alias like RUN_ALL for these--maybe RUN_TOR_TESTS.)

comment:3 Changed 3 months ago by teor

Cc: atagar added

I think that's a question for atagar?

comment:4 Changed 3 months ago by asn

Reviewer: → asn

comment:5 in reply to:  2 Changed 3 months ago by teor

Replying to nickm:

This looks reasonable to me, but I wonder if it will be stable long-term. If stem adds a new group of tests that we want to cover, will it necessarily be under control.controller, control.base_controller, or process ?

(That's not a blocking problem for this ticket, but it does suggest to me that we should ask stem to incorporate an alias like RUN_ALL for these--maybe RUN_TOR_TESTS.)

I created #31593 for this.

comment:6 Changed 3 months ago by nickm

Status: needs_review → merge_ready

comment:7 Changed 3 months ago by nickm

Keywords: dgoulet-merge added
Reviewer: asn → nickm
Status: merge_ready → needs_review

Setting myself as reviewer.

comment:8 Changed 3 months ago by nickm

Status: needs_review → merge_ready

comment:9 Changed 3 months ago by dgoulet

Milestone: Tor: 0.4.2.x-final → Tor: 0.4.0.x-final

Merged to 041 and master. Moving to 040 milestone for backport.

comment:10 Changed 3 months ago by teor

We need to merge this CI fix to avoid conflicts with #30901, so I'll do it as soon as CI is free and green.

comment:11 Changed 3 months ago by teor

Milestone: Tor: 0.4.0.x-final → Tor: 0.3.5.x-final
Resolution: → fixed
Status: merge_ready → closed

Merged to maint-0.3.5 and later.

Note: See TracTickets for help on using tickets.