search

decred / dcrd

Decred daemon in Go (golang).
https://decred.org
ISC License
731 stars 289 forks source link

addrmgr: Increase test coverage. #3402

Closed matthawkins90 closed 3 weeks ago

matthawkins90 commented 4 weeks ago

This PR is the first of more changes coming to Addrmgr.

However, in an attempt to create smaller PRs that are easier to review, this PR is simply increasing test coverage for the module without making any other changes.

The first commit has zero additions or subtractions--it's simply a reorganization to make the testfile easier to read and work on.

There are still multiple functions that don't have 100% test coverage, but in general those are unhittable cases. The only functions that I'm planning to increase coverage on (in a future PR) are AddressCache and newAddressFromString. Everything else I feel is covered to the best extent it can be.

Coverage before:

$ cd addrmgr
$ go test -coverprofile=coverage.out
PASS
coverage: 90.7% of statements
ok      github.com/decred/dcrd/addrmgr/v2       0.209s

Coverage after:

$ go test -coverprofile=coverage.out 
PASS
coverage: 94.5% of statements
ok      github.com/decred/dcrd/addrmgr/v2       0.205s

Details before:

$ go tool cover -func=coverage.out
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:230:   addOrUpdateAddress      88.2%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:309:   expireNew               66.7%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:352:   getOldestAddressIndex   100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:367:   getNewBucket            100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:388:   getTriedBucket          100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:408:   addressHandler          88.9%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:428:   savePeers               80.9%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:500:   loadPeers               90.9%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:519:   deserializePeers        72.3%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:616:   Start                   83.3%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:635:   Stop                    71.4%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:652:   AddAddresses            100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:664:   numAddresses            100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:672:   NeedMoreAddresses       100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:682:   AddressCache            77.8%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:723:   reset                   100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:750:   HostToNetAddress        100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:784:   GetAddress              97.1%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:844:   find                    100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:852:   Attempt                 100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:876:   Connected               100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:905:   Good                    91.3%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1002:  SetServices             100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1028:  AddLocalAddress         100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1054:  HasLocalAddress         100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1065:  LocalAddresses          100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1115:  getReachabilityFrom     100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1186:  GetBestLocalAddress     100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1226:  ValidatePeerNa          100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1237:  New                     100.0%
github.com/decred/dcrd/addrmgr/v2/error.go:20:          Error                   0.0%
github.com/decred/dcrd/addrmgr/v2/error.go:33:          Error                   0.0%
github.com/decred/dcrd/addrmgr/v2/error.go:38:          Unwrap                  0.0%
github.com/decred/dcrd/addrmgr/v2/error.go:43:          makeError               100.0%
github.com/decred/dcrd/addrmgr/v2/knownaddress.go:47:   NetAddress              100.0%
github.com/decred/dcrd/addrmgr/v2/knownaddress.go:54:   LastAttempt             100.0%
github.com/decred/dcrd/addrmgr/v2/knownaddress.go:63:   chance                  100.0%
github.com/decred/dcrd/addrmgr/v2/knownaddress.go:88:   isBad                   100.0%
github.com/decred/dcrd/addrmgr/v2/log.go:21:            UseLogger               0.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:36:     IsRoutable              100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:44:     ipString                100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:56:     Key                     100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:64:     String                  100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:70:     Clone                   100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:77:     AddService              100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:84:     newAddressFromString    71.4%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:99:     NewNetAddressIPPort     100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:97:        ipNet                   100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:102:       isIPv4                  100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:107:       isLocal                 100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:115:       isOnionCatTor           100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:131:       addressType             100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:150:       isRFC1918               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:161:       isRFC2544               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:167:       isRFC3849               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:173:       isRFC3927               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:179:       isRFC3964               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:185:       isRFC4193               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:191:       isRFC4380               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:197:       isRFC4843               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:203:       isRFC4862               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:210:       isRFC5737               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:222:       isRFC6052               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:228:       isRFC6145               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:234:       isRFC6598               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:242:       isValid                 100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:252:       IsRoutable              100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:264:       GroupKey                100.0%
total:                                                  (statements)            90.7%

Details after:

go tool cover -func=coverage.out   
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:230:   addOrUpdateAddress      94.1%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:309:   expireNew               100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:352:   getOldestAddressIndex   100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:367:   getNewBucket            100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:388:   getTriedBucket          100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:408:   addressHandler          88.9%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:428:   savePeers               85.1%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:500:   loadPeers               90.9%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:519:   deserializePeers        72.3%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:616:   Start                   83.3%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:635:   Stop                    71.4%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:652:   AddAddresses            100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:664:   numAddresses            100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:672:   NeedMoreAddresses       100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:682:   AddressCache            94.4%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:723:   reset                   100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:750:   HostToNetAddress        100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:784:   GetAddress              97.1%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:844:   find                    100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:852:   Attempt                 100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:876:   Connected               100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:905:   Good                    97.8%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1002:  SetServices             100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1028:  AddLocalAddress         100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1054:  HasLocalAddress         100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1065:  LocalAddresses          100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1115:  getReachabilityFrom     100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1186:  GetBestLocalAddress     100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1226:  ValidatePeerNa          100.0%
github.com/decred/dcrd/addrmgr/v2/addrmanager.go:1237:  New                     100.0%
github.com/decred/dcrd/addrmgr/v2/error.go:20:          Error                   100.0%
github.com/decred/dcrd/addrmgr/v2/error.go:33:          Error                   100.0%
github.com/decred/dcrd/addrmgr/v2/error.go:38:          Unwrap                  100.0%
github.com/decred/dcrd/addrmgr/v2/error.go:43:          makeError               100.0%
github.com/decred/dcrd/addrmgr/v2/knownaddress.go:47:   NetAddress              100.0%
github.com/decred/dcrd/addrmgr/v2/knownaddress.go:54:   LastAttempt             100.0%
github.com/decred/dcrd/addrmgr/v2/knownaddress.go:63:   chance                  100.0%
github.com/decred/dcrd/addrmgr/v2/knownaddress.go:88:   isBad                   100.0%
github.com/decred/dcrd/addrmgr/v2/log.go:21:            UseLogger               100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:36:     IsRoutable              100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:44:     ipString                100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:56:     Key                     100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:64:     String                  100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:70:     Clone                   100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:77:     AddService              100.0%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:84:     newAddressFromString    71.4%
github.com/decred/dcrd/addrmgr/v2/netaddress.go:99:     NewNetAddressIPPort     100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:97:        ipNet                   100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:102:       isIPv4                  100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:107:       isLocal                 100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:115:       isOnionCatTor           100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:131:       addressType             100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:150:       isRFC1918               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:161:       isRFC2544               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:167:       isRFC3849               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:173:       isRFC3927               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:179:       isRFC3964               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:185:       isRFC4193               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:191:       isRFC4380               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:197:       isRFC4843               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:203:       isRFC4862               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:210:       isRFC5737               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:222:       isRFC6052               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:228:       isRFC6145               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:234:       isRFC6598               100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:242:       isValid                 100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:252:       IsRoutable              100.0%
github.com/decred/dcrd/addrmgr/v2/network.go:264:       GroupKey                100.0%
total:                                                  (statements)            94.5%
matthawkins90 commented 4 weeks ago

I think that's a fair take, and yes in general I don't think it matters much. I had a few reasons for reorganizing this file in particular, though:

  1. I was looking for pointless and redundant tests. I was also trying to make sure I surveyed the tests we already have, because I don't want to write redundant tests.
  2. The test TestCorruptPeersFile which I later renamed as TestLoadPeersWithCorruptPeersFile was originally difficult for me to understand its purpose. All it calls is Start() and Stop(). But it's actually testing loadPeers, that if the call to deserializePeers fails, that loadPeers will correctly delete the corrupt peers file. So to help myself understand what the test was trying to accomplish, I moved it to where loadPeers is situated (near Start and Stop), renamed it, and added a test description. I dunno, it just makes sense to me to put similar tests near each other, and that sometimes calls for better organization.
matthawkins90 commented 3 weeks ago

Should I squash this down to two commits?

davecgh commented 3 weeks ago

Yes, please squash down to those 2. Should be good to go after that.