#21799 closed defect (fixed)

Unittest fail: FAIL ../tor/src/test/test_entrynodes.c:618: assert(smartlist_len(gs_br->sampled_entry_guards) OP_EQ 2): 1 vs 2

Reported by: asn Owned by:
Priority: High Milestone: Tor: 0.3.0.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: tor-guard
Cc: weasel Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

entrynodes/parse_from_state_full: [forking] 
  FAIL ../tor/src/test/test_entrynodes.c:618: assert(smartlist_len(gs_br->sampled_entry_guards) OP_EQ 2): 1 vs 2
  [parse_from_state_full FAILED]

The entry nodes unittests are failing because of an expired timestmap. Specifically:

  "Guard in=bridges rsa_id=5800000000000000000000000000000000000000 "
    "bridge_addr=37.218.246.143:28366 "
    "sampled_on=2016-11-18T15:07:34 sampled_by=0.3.0.0-alpha-dev listed=1\n";

since it was sampled over 120 days ago we get:

Mar 22 11:44:29.227 [info] sampled_guards_update_from_consensus(): Removing sampled guard          $5800000000000000000000000000000000000000 ($5800000000000000000000000000000000000000): it was sampled over 120 days ago, but never 
             confirmed.

The fix is to use get_yesterday_date_str() instead of a hardcoded date. It's not trivial tho because in the end the test tries to be smart and predict how the state is gonna be modified, so we need to also work on the tt_str_op(joined, OP_EQ, ...) part.

Also, there seem to be more unittests with hardcoded 2016 dates. We should see if those are ticking bombs as well.

Child Tickets

Change History (5)

comment:1 Changed 14 months ago by weasel

Cc: weasel added

comment:2 Changed 14 months ago by nickm_mobile

As an alternative fix, we could make the test set our view of the current time with update_approx_time(), so that the test runs 100 days ago forever. (We might need corresponding changes to entrynodes.c or bridges.c to look at approx_time() rather than time(NULL).)

comment:3 Changed 14 months ago by nickm_mobile

Status: newneeds_review

Bug 21799 in my public repository. How's that? It should apply cleanly to 0.3.0

comment:4 Changed 14 months ago by asn

Status: needs_reviewmerge_ready

Ha. Neat hack. Looks good to me and the test seems to pass alright now.

Maybe we could expand the comment to mention the date that 1481621834 represents just to make it clearer. (fwiw, it seems to be Tue, 13 Dec 2016 09:37:14 GMT)

comment:5 Changed 14 months ago by nickm_mobile

Resolution: fixed
Status: merge_readyclosed

Merged! Let's see if things are happy now.

Note: See TracTickets for help on using tickets.