Opened 2 years ago

Closed 2 years ago

#23310 closed defect (implemented)

test: prop224 client unit tests

Reported by: dgoulet Owned by:
Priority: Very High Milestone: Tor: 0.3.2.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-hs, prop224, test-unit
Cc: Actual Points:
Parent ID: #23300 Points:
Reviewer: Sponsor:

Description

We need to a LOT more client unit tests.

Child Tickets

Change History (11)

comment:1 Changed 2 years ago by asn

Pushed branch bug23310_client_intro_test in my repo which tests the client-side picking intro points, the intro state cache, etc.

It also found a subtle memleak which is fixed as part of the branch.

comment:2 Changed 2 years ago by dgoulet

More tests, including those in asn's branch above, can be found here: ticket23310_032_01

comment:3 Changed 2 years ago by asn

Pushed some improvements to the #23387 tests in my bug23310_hsdir_sync branch.
Let's include those too when we make the final branch here.

comment:4 Changed 2 years ago by asn

OK I think we now have enough unittests collected here that warrants its own patch and merge time.

Please check branch bug23310 in my repo for a branch based on latest master, that encorporates all the above branches together:
a) An expansion pack for test_client_service_hsdir_set_sync().
b) A unittest for HSDir index computation including a python test vector.
c) A unittest for clients picking intro points, plus a memleak fix (75d85d6078).
d) A unittest for turning link specifier sets to extend_info_t.

comment:5 Changed 2 years ago by asn

Status: newmerge_ready

Setting this to merge_ready I will soon compute new test coverage metrics for master.

comment:6 Changed 2 years ago by asn

Status: merge_readyneeds_review

Putting it on needs_review for some dgoulet validation.

comment:7 Changed 2 years ago by dgoulet

Status: needs_reviewneeds_revision

Ooooooook so #23502 has almost the equivalency of extend_info_from_lspecs in that branch, I didn't realized that I had made a test previously in this branch.....

So I suggest we remove that test from the branch. Furthermore, maybe can you rebase it on latest master?

comment:8 Changed 2 years ago by asn

Status: needs_revisionneeds_review

OK, removed unittest (d) from above based on dgoulet's suggestion and also rebased on latest master. Putting it back on needs_review so that dgoulet can inspect.

Version 0, edited 2 years ago by asn (next)

comment:9 Changed 2 years ago by dgoulet

I've fixed an issue that appeared with #23502 which is that an intro point can have an IPv6 only. Then also make happy check-spaces.

See my branch ticket23310_032_02

comment:10 Changed 2 years ago by asn

Status: needs_reviewmerge_ready

LGTM dgoulet!

comment:11 Changed 2 years ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged!

Note: See TracTickets for help on using tickets.