[test] Add Unit Test for GetListenPort #10278

pull jimmysong wants to merge 1 commits into bitcoin:master from jimmysong:test_listenport changing 1 files +13 −0

jimmysong commented at 2:28 PM on April 25, 2017: contributor

Add very basic unit test for GetListenPort in net_tests.cpp
jimmysong commented at 2:29 PM on April 25, 2017: contributor

Please let me know if this is too basic a unit test. I'm using unit test writing as a way to get familiar with the code base.
laanwj added the label Tests on Apr 25, 2017
laanwj commented at 4:03 PM on April 25, 2017: member

This tests only one code path of GetListenPort. Possibly this could be extended with a test for the -port argument as well, e.g. temporarily set the option to something else and see if the returned value changes to that.

Anyhow, thanks for adding a test.
laanwj commented at 4:05 PM on April 25, 2017: member

Also might make sense to compare against Params().GetDefaultPort() instead of hardcoding the number.

[test] Add Unit Test for GetListenPort

Add very basic unit test for GetListenPort in net_tests.cpp

1b144495d0

jimmysong force-pushed on Apr 25, 2017
jimmysong commented at 5:17 PM on April 25, 2017: contributor

Thanks for the review @laanwj, I've changed the test per your suggestions and squashed.
laanwj merged this on Apr 26, 2017
laanwj closed this on Apr 26, 2017
laanwj referenced this in commit 8254a8ae21 on Apr 26, 2017
in src/test/net_tests.cpp:83 in 1b144495d0
```
  78 | +    // test default
  79 | +    unsigned short port = GetListenPort();
  80 | +    BOOST_CHECK(port == Params().GetDefaultPort());
  81 | +    // test set port
  82 | +    unsigned short altPort = 12345;
  83 | +    SoftSetArg("-port", std::to_string(altPort));
```
MarcoFalke commented at 11:13 AM on May 9, 2017:

This will modify the global state of test run after this test case. There are other places in the test code where this already happens, but imo, unit tests should really be unit test.

jimmysong commented at 1:53 PM on May 9, 2017:

Thanks for the feedback!

What should I do in the future? Unset the arg afterwards?

MarcoFalke commented at 2:44 PM on May 9, 2017:

Yes, I think this is reasonable for tests.
MarcoFalke commented at 11:18 AM on May 9, 2017: member

Not sure if implementing unit tests for functions by effectively copy pasting the function into another cpp file and checking equality for a one or two input arguments is really worth it. In case GetListenPort breaks, I assume all code that depends on GetListenPort breaks as well.

Though, you mentioned that you wanted to get familiar with writing tests and this is already merged, so fine with me.
PastaPastaPasta referenced this in commit 221e95ca08 on Jun 10, 2019
PastaPastaPasta referenced this in commit e492f29bd0 on Jun 10, 2019
PastaPastaPasta referenced this in commit 346f1d2067 on Jun 10, 2019
PastaPastaPasta referenced this in commit dd2922ea8b on Jun 11, 2019
PastaPastaPasta referenced this in commit e2552515dd on Jun 11, 2019
PastaPastaPasta referenced this in commit 658a3e6052 on Jun 12, 2019
PastaPastaPasta referenced this in commit e22ef596b1 on Jun 14, 2019
PastaPastaPasta referenced this in commit 1dec8e6933 on Jun 14, 2019
PastaPastaPasta referenced this in commit 4624f97acf on Jun 15, 2019
PastaPastaPasta referenced this in commit b66ba12672 on Jun 19, 2019
barrystyle referenced this in commit 7b90b6dfed on Jan 22, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels