Opened 3 months ago

Closed 2 months ago

#30676 closed defect (implemented)

From test-stem coverage, infer which events and commands are not getting covered

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version:
Severity: Normal Keywords: 041-should
Cc: Actual Points:
Parent ID: Points:
Reviewer: atagar Sponsor:

Description


Child Tickets

Change History (11)

comment:1 Changed 3 months ago by nickm

(Then tell atagar, along with possibly a list of useful inputs/assertoins, and he might write some tests for them :) )

comment:2 Changed 3 months ago by nickm

Component: - Select a componentCore Tor/Tor
Keywords: 041-should added
Owner: set to nickm
Status: newaccepted

comment:3 Changed 3 months ago by nickm

https://people.torproject.org/~nickm/volatile/coverage-stem-20190528.tar.xz has coverage here with "make test-stem", and with "make check test-stem".

comment:4 Changed 3 months ago by nickm

I instrumented tor to log all the commands that it receives from make test-stem. Here is a list, with counts:

    105 ADD_ONION
    138 AUTHCHALLENGE
    498 AUTHENTICATE
     12 blarg
     49 DEL_ONION
    517 GETCONF
   1436 GETINFO
      7 invalid
     35 +LOADCONF
    590 PROTOCOLINFO
    106 RESETCONF
     14 SAVECONF
    245 SETCONF
    328 SETEVENTS
     28 SIGNAL
      8 TAKEOWNERSHIP

That means that the following commands are the ones that are currently getting zero testing from test-stem:

ATTACHSTREAM
CLOSECIRCUIT
CLOSESTREAM
DROPGUARDS
DROPOWNERSHIP
EXTENDCIRCUIT
HSFETCH
HSPOST
MAPADDRESS
POSTDESCRIPTOR
REDIRECTSTREAM
RESOLVE
SETCIRCUITPURPOSE
USEFEATURE
Last edited 3 months ago by nickm (previous) (diff)

comment:5 Changed 3 months ago by atagar

Hi Nick. I'd be delighted to add coverage for those commands if you have test scenarios you'd care to propose.

That said, in most of their cases (ATTACHSTREAM, HSFETCH, etc) the command requires a live network connection, which are not ran by default due to their long runtime and potential for false positives. In fact, in some of these cases Stem *does* have coverage, it's not getting invoked because Travis doesn't include the '--target ONLINE' argument. For example, here's our HSFETCH test...

https://gitweb.torproject.org/stem.git/tree/test/integ/control/controller.py#n1282

Teor is helping me migrate to Travis (#30653) and I requested for us to run online tests as part of Stme's CI. If tor would like to run online stem's online tests as well that will increase coverage, but come at the cost of runtime and added chance of false positives.

comment:6 Changed 3 months ago by teor

We thought about adding tor unit tests for the hs fetch functions in #29165, they would have caught this bug.

I ran ONLINE tests in stem's CI, they also found this bug, see #30697.

comment:7 Changed 2 months ago by nickm

Okay, I have a shorter list now, based on "make test-stem-full", which includes the ONLINE tests.

dropguards
dropownership
hspost
mapaddress
postdescriptor
redirectstream
resolve

If this list is accurate, we can start sketching how how to do integration tests for these commands (in another ticket).

comment:8 Changed 2 months ago by nickm

Reviewer: atagar
Status: acceptedneeds_review

Atagar, if you can confirm that the list above is right, I can start opening new tickets for test scenarios.

comment:9 Changed 2 months ago by atagar

Status: needs_reviewnew

Yup! None of those commands have tests at present. Test scenarios welcome.

comment:10 Changed 2 months ago by nickm

Status: newaccepted

comment:11 Changed 2 months ago by nickm

Resolution: implemented
Status: acceptedclosed

Okay, the work here is done. I've opened #30828 to design tests for the untested commands, and open tickets for implementing those tests.

Note: See TracTickets for help on using tickets.