Opened 23 months ago
Last modified 20 months ago
#25118 needs_revision defect
We need circuit launch and cannibalization unit tests
Reported by: | dgoulet | Owned by: | |
---|---|---|---|
Priority: | High | Milestone: | Tor: unspecified |
Component: | Core Tor/Tor | Version: | |
Severity: | Normal | Keywords: | tor-circuit, tor-test, 034-triage-20180328, 034-removed-20180328 |
Cc: | Actual Points: | ||
Parent ID: | Points: | ||
Reviewer: | Sponsor: |
Description
In particular, these functions circuit_launch_by_extend_info()
and circuit_find_to_cannibalize()
really needs unit tests!
They do so much and they are so core that we need to assess what they are doing is ok. See #24469 for an example of what it should have done.
Child Tickets
Change History (5)
comment:1 Changed 21 months ago by
Keywords: | 034-triage-20180328 added |
---|
comment:2 Changed 21 months ago by
Keywords: | 034-removed-20180328 added |
---|
comment:3 Changed 20 months ago by
Milestone: | Tor: 0.3.4.x-final → Tor: unspecified |
---|
These tickets, tagged with 034-removed-*, are no longer in-scope for 0.3.4. We can reconsider any of them, if time permits.
comment:4 Changed 20 months ago by
Status: | new → needs_review |
---|
I have been working on a test. I think I came with a good approach but the test is giving me an error. I don't know if the function tested is just not working properly or the test is wrong.
Please check my github branch bug25118! Thank you =)
comment:5 Changed 20 months ago by
Status: | needs_review → needs_revision |
---|
Ok so basically here is the problem:
May 04 09:31:55.761 [debug] circuit_launch_by_extend_info(): Haven't fetched enough directory info yet; canceling circuit launch.
A useful trick for you is to use ./src/test/test <your-test> --debug
which will output the debug logs of the test. You can see that you don't get at all in the function but rather exit very fast with a NULL.
So couple of things:
- Use
tt_assert(new_circ)
to make sure you do have a circuit that you expected.
router_have_minimum_dir_info()
is mockable so you'll have to mock it to return true.
This circuit_launch_by_extend_info()
is really not simple to unit test, you might need to mock lots of things. So instead, you could skip it and just unit tests circuit_find_to_cannibalize()
which it self will be something intense as well.
Per our triage process, these tickets are pending removal from 0.3.4.