General improvements on READMEs for unit tests and functional tests:
src/test folder to the top0 | @@ -1,3 +1,25 @@ 1 | +# Unit tests 2 | + 3 | +The sources in this directory are unit test cases. Boost includes a 4 | +unit testing framework, and since bitcoin already uses Boost, it makes
Nit: s/bitcoin/Bitcoin Core/
done
6 | +configure some other framework (we want as few impediments to creating 7 | +unit tests as possible). 8 | + 9 | +The build system is setup to compile an executable called `test_bitcoin` 10 | +that runs all of the unit tests. The main source file is called 11 | +setup_common.cpp.
Nit: Use setup_common.cpp instead of "setup_common.cpp".
done
199 | 200 | -use the directory path to get the pid from the pid file: 201 | +Then you can use the pid to start `gdb`: 202 | 203 | ```bash 204 | -cat /tmp/user/1000/testo9vsdjo3/node1/regtest/bitcoind.pid
Why remove? Is this way no longer possible?
It still works but getting the pid from within pdb is just much simpler. But I can add it back if we want to keep it in.
Not everyone is running with pdb, so to keep the fallback would be nice
Hm, just curious: is there another practical way to pause a functional test besides pdb? Anyway, since it doesn't hurt, I added it back now :)
I rarely use pdb but often add some manual input("Pause") breakpoints. (while running the test directly, not via test_runner).
@mzumsande I see. Thanks!
import time; time.sleep(10000);
LGTM
ACK
72 | -one test file for each class or source file for which you want to create 73 | -unit tests. The file naming convention is `<source_filename>_tests.cpp` 74 | -and such files should wrap their tests in a test suite 75 | -called `<source_filename>_tests`. For an example of this pattern, 76 | -examine `uint256_tests.cpp`. 77 | +For debugging you can launch the test_bitcoin executable with `gbd`or `lldb` and
typo: gdb
done
13 | +### Note on adding test cases 14 | + 15 | +To add a new unit test file to our test suite you need 16 | +to add the file to `src/Makefile.test.include`. The pattern is to create 17 | +one test file for each class or source file for which you want to create 18 | +unit tests. The file naming convention is `<source_filename>_tests.cpp`
typo: 2 spaces before "The"
done
Concept ACK, found two minor typos.
One thing I have been missing in the unit test logging description before is whether there is a good way to view the LogPrint() messages that go to debug.log when they are invoked from a unit test.
One thing I have been missing in the unit test logging description before is whether there is a good way to view the LogPrint() messages that go to debug.log when they are invoked from a unit test.
In theory they are written to the debug.log in the temp data dir (however that dir is deleted when the destructor of the testing setup is called).
Usually I use LogInstance().m_print_to_console = 1; // TODO remove when debugging a test. Maybe that should be exposed by an env var?
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
One thing I have been missing in the unit test logging description before is whether there is a good way to view the LogPrint() messages that go to debug.log when they are invoked from a unit test.
In theory they are written to the
debug.login the temp data dir (however that dir is deleted when the destructor of the testing setup is called).Usually I use
LogInstance().m_print_to_console = 1; // TODO removewhen debugging a test. Maybe that should be exposed by an env var?
I also thought about this some time ago but did not find a simple way to do this. I will keep Marco's suggestion in mind for a follow-up!
ACK afdcfb237e0d86f65eaceff9c3abc181c6f5576e
<details><summary>Show signature and timestamp</summary>
Signature:
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512
ACK afdcfb237e0d86f65eaceff9c3abc181c6f5576e
-----BEGIN PGP SIGNATURE-----
iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUixVgv+IiRmzeRMPczLxZhkhHVl+Q8rzkoEJbz79Ph/CZmUXz94LhDGyPiE8Qhf
5FiLvLWBJXtDCYSoepjRKL6U32n4u6KOMRy/qxsy4OoaAq3zeTczk+nzsr/sEw0z
f5hCWnezEC9Pp9rjV84t5ycmgm83z+HBGnSF+Kp+2PVbdezN470SHSpNpRO9YAUI
DRFV5PF51sm++lOz7NLEwGkQt8sND4KmRhsyp+vnjMCPmF8ZvP3MiJ9+O93DiSKx
xv+vzTi0jjUrgDPhMGyIz4w7PG1gB6bB022NGro74aeMLRNspooDL7c/m3SkBwy3
KDwL36dAu4HF78VuKWCJLMqWZAoMQXKWqCeh48XF/OOmFpAill6xE3OpfkRJrGvy
RnM8V8Dd1qPPpgfLDc/Kv882hObQQFlJOJBC2QyQ/PboGLPVDnjQRoKubJ9judCj
vQERZz6Mpml67HrrmkIF6Qe3MNDyn/f3M/zgYr9w16HfXakp8mz8dFgJ9sdRmK/f
RGTBs7hd
=8aLm
-----END PGP SIGNATURE-----
Timestamp of file with hash 6cad13c8512fec76fb404b4bec84bdafcb7d4e2a901dcb67122c03aa41764d54 -
</details>
16 | +to add the file to `src/Makefile.test.include`. The pattern is to create 17 | +one test file for each class or source file for which you want to create 18 | +unit tests. The file naming convention is `<source_filename>_tests.cpp` 19 | +and such files should wrap their tests in a test suite 20 | +called `<source_filename>_tests`. For an example of this pattern, 21 | +examine `uint256_tests.cpp`.
suggestion: s/examine/see/
done
8 | + 9 | +The build system is setup to compile an executable called `test_bitcoin` 10 | +that runs all of the unit tests. The main source file is called 11 | +`setup_common.cpp`. 12 | + 13 | +### Note on adding test cases
Perhaps place the "Adding test cases" after the other sections, as it is perhaps less frequently needed compared to running tests or debugging them.
done
195 | +(pdb) self.node[1].process.pid 196 | +``` 197 | + 198 | +Alternatively, you can find the pid by inspecting the temp folder for the specific test 199 | +you are running. The path to that folder is printed at the beginning of every 200 | +test execution:
suggestion: s/test execution/test run/
done
0 | @@ -1,3 +1,25 @@ 1 | +# Unit tests 2 | + 3 | +The sources in this directory are unit test cases. Boost includes a 4 | +unit testing framework, and since Bitcoin Core already uses Boost, it makes 5 | +sense to simply use this framework rather than require developers to 6 | +configure some other framework (we want as few impediments to creating 7 | +unit tests as possible). 8 | + 9 | +The build system is setup to compile an executable called `test_bitcoin`
nit: a "setup" is a noun, "set up" is the verb version which should be used here, e.g. "The build system is set up to compile..."
done
ACK modulo a few suggestions below. Thanks for improving the test docs!
ACK afdcfb237e0d86f65eaceff9c3abc181c6f5576e. Nice additions.
ACK 43e7d576f590e90ad7d1ba3d550671a7958f1188