Creates unittests for addrman, makes addrman more testable. #6720

1. 
   eth0 commented at 3:55 pm on September 24, 2015: none

   • Creates unittests for addrman, while not all addrman behavior is covered by these tests they provide a useful starting place and increase code coverage.
   • Minor modifications to addrman to allow deterministic and effective testing.
2. laanwj added the label Tests on Sep 24, 2015
3. in src/addrman.h: in 554903bd8d outdated

   568@@ -567,6 +569,14 @@ class CAddrMan
   569             Check();
   570         }
   571     }
   572+
   573+
   574+    #ifdef __ADDRMAN_TEST__
   

   ---

   ---

   

   jonasschnelli commented at 4:07 pm on September 24, 2015:

   Would it make sense to link this to the configure flag --enable-tests?
4. 
   eth0 commented at 5:04 pm on September 24, 2015: none

   @jonasschnelli What advantages are offered by using the –enable-tests flag?

   Also how would one learn the value of –enable-tests at the level necessary to change code behavior? I suppose you could move the MakeDeterministic method into its own source file and then only include that source when building tests but that is a bit complicated.

   I could also remove the ifdef ADDRMAN_TEST check completely.

5. 
   jonasschnelli commented at 6:16 pm on September 24, 2015: contributor

   @eth0: maybe remote the #ifdef __ADDRMAN_TEST__ completely or export the ENABLE_TESTS over AC_DEFINE_UNQUOTED() in configure.ac and use this define to determine if you compile the MakeDeterministic() function?

   Is the newOnly related to the new unit-tests?

   Nice work! Concept ACK.

6. Creates unittests for addrman, makes addrman testable.

   Adds several unittests for addrman to verify it works as expected.
   Makes small modifications to addrman to allow deterministic and targeted tests.

   1534d9a83c
7. 
   eth0 commented at 7:28 pm on September 24, 2015: none

   @jonasschnelli
   I removed the macro completely.

   newOnly is related to the unit-tests since I need a way to test if an addr is in the new or tried tables.

   I am developing some non-unittest code which builds on the newOnly functionality but it is independent of this pull-request.

8. 
   dcousens commented at 6:46 am on September 25, 2015: contributor

   concept ACK, non test-code utACK
9. 
   EthanHeilman commented at 5:42 pm on September 30, 2015: contributor

   @dcousens Anything I can do to move this along? I’ve tested this code in the Bitcoin client and the unittests all pass.
10. 
    jgarzik commented at 9:46 am on October 1, 2015: contributor

    ut ACK - doesn’t test very much, but what it does test, looks correct based on quick review :)
11. laanwj merged this on Oct 7, 2015
12. laanwj closed this on Oct 7, 2015

    ---

13. laanwj referenced this in commit d479311dba on Oct 7, 2015
14. 
    laanwj commented at 1:00 pm on October 7, 2015: member

    utACK
15. luke-jr referenced this in commit 7a9724adb8 on Jan 9, 2016
16. MarcoFalke locked this on Sep 8, 2021

Contributors
eth0 jonasschnelli dcousens EthanHeilman jgarzik laanwj

Labels
Tests