c/ooni-probe-cli
Watch
1
0
Fork
You've already forked ooni-probe-cli
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

fix(netx): repair BogonResolver tests (#401)

Browse source 
The BogonResolver relied on its wrapper resolver to pass along the
list of addresses _and_ the error. But the idiomatic thing to do is
often to return `nil` when there is an error.

I broke this very fragile assumption in https://github.com/ooni/probe-cli/pull/399.

I could of course fix it, but this assumption is clearly wrong
and we should not allow such fragile code in the tree.

We are not using BogonIsError much in the tree. The only place in
which we're using it for measuring seems to be dnscheck.

It may be that this surprising behavior was what caused the issue at
https://github.com/ooni/probe/issues/1510 in the first place.

Regardless, let's remove fragile code and adjust the test that was
failing. Also that test is quick so it can run in `-short` mode.

Spotted while working on https://github.com/ooni/probe/issues/1505.
This commit is contained in:
Simone Basso 2021-06-25 11:51:10 +02:00 • committed by GitHub
commit acef18a955
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 11 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

7
internal/engine/netx/integration_test.go
View file

@ -68,9 +68,6 @@ func TestSuccess(t *testing.T) {
}
func TestBogonResolutionNotBroken(t *testing.T) {
if testing.Short() {
t.Skip("skip test in short mode")
}
saver := new(trace.Saver)
r := netx.NewResolver(netx.Config{
BogonIsError: true,
@ -87,7 +84,7 @@ func TestBogonResolutionNotBroken(t *testing.T) {
if err.Error() != errorx.FailureDNSBogonError {
t.Fatal("error not correctly wrapped")
}
if len(addrs) != 1 || addrs[0] != "127.0.0.1" {
t.Fatal("address was not returned")
if len(addrs) > 0 {
t.Fatal("expected no addresses here")
}
}