Unit test address/get_if_addrs6_list_no_internal should succeed if there are only internal addresses

Split off #19960 (moved):

It is ok for the log message generated by address/get_if_addrs6_list_no_internal to be either:

• "connect() failed", or
• "unable to create socket", or
• "Address that we determined via UDP socket magic is unsuitable for public comms."

But we don't currently allow the third option.

address/get_if_addrs6_list_no_internal: [forking] 
  FAIL src/test/test_address.c:850: expected log to contain "connect() failed" o
r "unable to create socket"  Captured logs:
    1. err: "Address that we determined via UDP socket magic is unsuitable for p
ublic comms.\n"

  [get_if_addrs6_list_no_internal FAILED]

changed milestone to %Tor: 0.2.9.x-final

added CoreTorTeam201611 component::core tor/tor easy intro milestone::Tor: 0.2.9.x-final points::0.1 priority::medium resolution::fixed severity::normal status::closed test type::defect version::tor 0.2.8.1-alpha labels

Trac:
0001-Add-error-checking-for-error-Address-that-we-determi.patch

Patch to add error for "Address that we determined via UDP socket magic is unsuitable for public comms."

Trac:
tor_test_output.log

Log file for my patch for "Address that we determined via UDP socket magic is unsuitable for public comms."

I have created a patch for this. Tell me what you think about this.

Thanks, Neel Chauhan

Replying to neel:

I have created a patch for this. Tell me what you think about this. ...

Thanks for this patch, neel!

Here's how I would make the patch better:

• I would keep expect_log_msg_containing_either as a 2-argument version, and add another macro expect_log_msg_containing_either3 for the 3-argument version. That way, we can use whichever one we need in future tests.
• I just read get_interface_address6_via_udp_socket_hack, and there are actually 4 things that can go wrong. So let's cover them all - the extra one is:
  • "getsockname() to determine interface failed" (I guess that means we need a 4-argument macro expect_log_msg_containing_either4 as well.)

Can you make these changes, and submit another patch?

Trac:
Status: new to needs_revision

Trac:
tor_patch_get_if_addrs6.patch

Updated patch

I uploaded an updated patch as tor_patch_get_if_addrs6.patch​.

-Neel

Hi Neel, thanks, almost there!

Both tests can fail for all 4 reasons. Can you put all 4 reasons in both tests?

Thanks!

Trac:
tor_patch_get_if_addrs6_2.patch

Patch with getsockname() error reason to test_address_get_if_addrs6_list_internal()

I added an updated patch (again).

-Neel

Ok, two typos, and then I think we're good to go:

• expect_log_msg_containing_either4 checks str3 twice, and doesn't check str4.
• test_address_get_if_addrs6_list_internal uses expect_log_msg_containing_either3 when it should use expect_log_msg_containing_either4

Trac:
tor_patch_get_if_addrs6_3.patch

Cleaned up patch with all four error messages and (hopefully) no typos.

Ok, I added a fixup commit for typos, whitespace, and line length; and a changes file, then I put it on a branch on github: https://github.com/teor2345/tor/commits/bug20634_029

Our coding standards about whitespace, line length, and changes files are in: https://gitweb.torproject.org/tor.git/tree/doc/HACKING/CodingStandards.md

For whitespace and line length, I usually just run our automated check: make check-spaces

If you're happy with the fixup and the changes file, let me know, and I will put the branch on the merge queue.

Thanks for helping out!

Trac:
Status: needs_revision to needs_review

Trac:
Keywords: easy intro test deleted, easy intro test CoreTorTeam201611 added

I'm fine with it.

-Neel

Please merge bug20634_029 in my github repository to 0.2.9, if possible.

It's a unit test fix, so I think it can go in after the feature freeze. (The bug has been around since 0.2.8.1-alpha, so it's not a regression.)

Trac:
Status: needs_review to merge_ready

Merging to 0.2.9 and forward. Thanks, Neel! Thanks Teor!

Trac:
Resolution: N/A to fixed
Status: merge_ready to closed

closed

changed time estimate to 48m

moved to tpo/core/tor#20634 (closed)