Opened 5 years ago

Closed 4 weeks ago

#6298 closed defect (fixed)

Tests should not depend on localhost==127.0.0.1 (Test 'addr/basic' failure in tor master)

Reported by: andrea Owned by: nickm
Priority: Low Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version: Tor: unspecified
Severity: Normal Keywords: tor-relay testing crapnet unit-tests easy
Cc: Actual Points: .0
Parent ID: Points: 1
Reviewer: Sponsor:

Description

I just built branch 'master' (revision 46434ecf5b6f1ad88deb86fdac044c41ae4b534b) from the tor.git repository with gcc 4.5.3 on x86_64-linux using this configure line:

CC="gcc -m64 -mtune=core2" PKG_CONFIG_LIBDIR=/usr/lib64/pkgconfig PKG_CONFIG_PATH=/usr/share/pkgconfig ./configure --prefix=/usr --libdir=/usr/lib64 --libexecdir=/usr/libexec64 --sysconfdir=/etc --localstatedir=/var --build=x86_64-linux --host=x86_64-linux --disable-linker-hardening --disable-asciidoc

This causes a test suite failure I haven't seen before:

addr/basic:

FAIL test_addr.c:36: assert((u32) == (0x7f000001u)): 2851995905 vs 2130706433
[basic FAILED]

Turns out the DNS in this hotel *resolves localhost to 169.254.1.1*. This is horribly wrong, of course, and I should have had a proper /etc/hosts for it anyway (*lashes self with wet noodle*), but why does the test suite depend on stuff random machines elsewhere on the network do at all to pass? Shouldn't we mock this stuff somehow?

Child Tickets

Change History (9)

comment:1 Changed 5 years ago by nickm

Agreed that unit tests shouldn't hit the network, and should work even on this crappy hotel internet.

One option here is to take the part of that test that hits the network and make it disabled-by-default. I added a feature to the upstream tinytest library last week that lets you mark some tests as not on-by-default, and then turn them on by saying @network_dependent or @takes_too_long or whatever. If that seems like a good choice, we can copy in the latest tinytest, take all the tests that fail in the hotel, or when you turn your laptop's wireless off, and mark them as disabled, and add an @alias to turn them back on.

Mocking is also maybe a good choice, though we haven't done it yet. Are there any good C mocking libraries/tools/systems/methodologies we should be looking at?

comment:2 Changed 5 years ago by nickm

  • Keywords tor-relay added

comment:3 Changed 5 years ago by nickm

  • Component changed from Tor Relay to Tor

comment:4 Changed 8 months ago by arma

  • Severity set to Normal

Does this bug remain, after all of our unit test work lately?

comment:5 Changed 8 months ago by teor

I'm pretty sure we still rely on localhost being present and sane.

I think I wrote additional tests that rely on localhost being present on local interfaces, although our IPv6-only testing probably weeded some of them out.

comment:6 Changed 5 weeks ago by nickm

  • Actual Points set to .0
  • Keywords testing crapnet unit-tests easy added
  • Milestone changed from Tor: unspecified to Tor: 0.3.1.x-final
  • Owner set to nickm
  • Points set to 1
  • Status changed from new to accepted
  • Summary changed from Test 'addr/basic' failure in tor master to Tests should not depend on localhost==127.0.0.1 (Test 'addr/basic' failure in tor master)

I munged my /etc/hosts file so that localhost pointed somewhere wrong, and then I made this test pass. No other tests needed fixing.

comment:7 Changed 5 weeks ago by nickm

  • Status changed from accepted to needs_review

Patch in my branch bug6298. Mocking made this much easier.

comment:8 Changed 4 weeks ago by ahf

  • Status changed from needs_review to merge_ready

LGTM.

comment:9 Changed 4 weeks ago by nickm

  • Resolution set to fixed
  • Status changed from merge_ready to closed

thanks for the review; merging!

Note: See TracTickets for help on using tickets.