| Message ID | 20231103111102.2801624-1-thaller@redhat.com |
|---|---|
| Headers | show |
| Series | add infrastructure for unit tests | expand |
On Fri, Nov 03, 2023 at 12:05:42PM +0100, Thomas Haller wrote: > There are new new make targets: > > - "build-all" > - "check" (runs "normal" tests, like unit tests and "tools/check-tree.sh"). > - "check-more" (runs extra tests, like "tests/build") > - "check-all" (runs "check" + "check-more") > - "check-local" (a subset of "check") > - "check-TESTS" (the unit tests) > > The unit tests have a test runner "tools/test-runner.sh". See > `tools/test-runner.sh -h` for options, like valgrind, GDB, or make. > It also runs the test in a separate namespace (rootless). > > To run unit tests only, `make check-TESTS` or `tools/test-runner.sh > tests/unit/test-libnftables-static -m`. > > The unit tests are of course still empty. "tests/unit" is the place > where tests shall be added. Thanks a lot for improving tests infrastructure.
Thomas Haller <thaller@redhat.com> wrote: Thanks for sending an initial empty skeleton. > There are new new make targets: > > - "build-all" > - "check" (runs "normal" tests, like unit tests and "tools/check-tree.sh"). > - "check-more" (runs extra tests, like "tests/build") > - "check-all" (runs "check" + "check-more") > - "check-local" (a subset of "check") > - "check-TESTS" (the unit tests) "check-unit" perhaps? TESTS isn't very descriptive. Also, why CAPS? If this is some pre-established standard, then maybe just document that in the commit changelog. Please don't do anything yet and wait for more comments, but I would prefer 'make check' to run all tests that we have. I would suggest - "check" (run all tests) - "check-unit" (the unit tests only) - "check-shell" (shell tests only) - "check-py" (python based tests only) - "check-json" (python based tests in json mode and json_echo) ... and so on.
On Fri, 2023-11-03 at 13:26 +0100, Florian Westphal wrote: > Thomas Haller <thaller@redhat.com> wrote: > > Thanks for sending an initial empty skeleton. > > > There are new new make targets: > > > > - "build-all" > > - "check" (runs "normal" tests, like unit tests and "tools/check- > > tree.sh"). > > - "check-more" (runs extra tests, like "tests/build") > > - "check-all" (runs "check" + "check-more") > > - "check-local" (a subset of "check") > > - "check-TESTS" (the unit tests) > > "check-unit" perhaps? TESTS isn't very descriptive. Also, > why CAPS? If this is some pre-established standard, then maybe just > document that in the commit changelog. "TESTS" is an autotools/automake thing. "check-TESTS" runs the tests that are setup via "TESTS=". AFAIU, with autotools, `make check` basically runs the targets `make check-local` and `make check-TESTS`. Anyway, I can just alias it via: check-unit: check-TESTS Note that other targets are currently called `make check-tests-build` `make check-tests-shell` (patch not yet send). It reminds of the directories "tests/{build,shell,unit}" and would lead to a name `make check-tests-unit`. But reconsidering, in v2 I will drop this "check-tests-" prefix then and call them just "check-{unit,build,shell}". > > Please don't do anything yet and wait for more comments, but > I would prefer 'make check' to run all tests that we have. currently `make check-more` only entails `make check-tests-build` (i.e. `tests/build/run-tests.sh`). Note that - `tests/build/run-tests.sh` calls `make distcheck` - `make distcheck` runs `make check`. I don't think it would make sense, to include the build check in `make check`. So I think there is a (small) place for "check-more" (and thus "check- all"). But yes, probably most other tests should be included by the common `make check`! > > I would suggest > - "check" (run all tests) > - "check-unit" (the unit tests only) > - "check-shell" (shell tests only) > - "check-py" (python based tests only) > - "check-json" (python based tests in json mode and json_echo) > > ... and so on. > ACK. Will send in v2.
On Fri, Nov 03, 2023 at 01:26:41PM +0100, Florian Westphal wrote: > Thomas Haller <thaller@redhat.com> wrote: > > Thanks for sending an initial empty skeleton. > > > There are new new make targets: > > > > - "build-all" > > - "check" (runs "normal" tests, like unit tests and "tools/check-tree.sh"). > > - "check-more" (runs extra tests, like "tests/build") > > - "check-all" (runs "check" + "check-more") > > - "check-local" (a subset of "check") > > - "check-TESTS" (the unit tests) > > "check-unit" perhaps? TESTS isn't very descriptive. Also, > why CAPS? If this is some pre-established standard, then maybe just > document that in the commit changelog. > > Please don't do anything yet and wait for more comments, but > I would prefer 'make check' to run all tests that we have. We had a few tests that have been shown to be unstable. I just would like that I don't hit this when making the release and hold back a release because a test fails occasionally. If we go for `make check' then all test runs must be reliable.
On Fri, 2023-11-03 at 16:33 +0100, Pablo Neira Ayuso wrote: > On Fri, Nov 03, 2023 at 01:26:41PM +0100, Florian Westphal wrote: > > Thomas Haller <thaller@redhat.com> wrote: > > > > Thanks for sending an initial empty skeleton. > > > > > There are new new make targets: > > > > > > - "build-all" > > > - "check" (runs "normal" tests, like unit tests and > > > "tools/check-tree.sh"). > > > - "check-more" (runs extra tests, like "tests/build") > > > - "check-all" (runs "check" + "check-more") > > > - "check-local" (a subset of "check") > > > - "check-TESTS" (the unit tests) > > > > "check-unit" perhaps? TESTS isn't very descriptive. Also, > > why CAPS? If this is some pre-established standard, then maybe just > > document that in the commit changelog. > > > > Please don't do anything yet and wait for more comments, but > > I would prefer 'make check' to run all tests that we have. > > We had a few tests that have been shown to be unstable. > > I just would like that I don't hit this when making the release and > hold back a release because a test fails occasionally. > > If we go for `make check' then all test runs must be reliable. > Agree. Tests must be reliable and `make distcheck/check` usable! Unstable tests must be fixed. It's a never-ending fight to keep the testsuite passing well enough. ATM, the reliability is not great, but not terrible either. Seems manageable to me. Thomas
There are new new make targets: - "build-all" - "check" (runs "normal" tests, like unit tests and "tools/check-tree.sh"). - "check-more" (runs extra tests, like "tests/build") - "check-all" (runs "check" + "check-more") - "check-local" (a subset of "check") - "check-TESTS" (the unit tests) The unit tests have a test runner "tools/test-runner.sh". See `tools/test-runner.sh -h` for options, like valgrind, GDB, or make. It also runs the test in a separate namespace (rootless). To run unit tests only, `make check-TESTS` or `tools/test-runner.sh tests/unit/test-libnftables-static -m`. The unit tests are of course still empty. "tests/unit" is the place where tests shall be added. Thomas Haller (6): gitignore: ignore build artifacts from top level file build: add basic "check-{local,more,all}" and "build-all" make targets build: add `make check-tests-build` to add build test build: add check for consistency of source tree build: cleanup if blocks for conditional compilation tests/unit: add unit tests for libnftables .gitignore | 15 +- Makefile.am | 128 ++++++++++++--- src/.gitignore | 5 - tests/unit/nft-test.h | 14 ++ tests/unit/test-libnftables-static.c | 16 ++ tests/unit/test-libnftables.c | 21 +++ tools/test-runner.sh | 228 +++++++++++++++++++++++++++ 7 files changed, 399 insertions(+), 28 deletions(-) create mode 100644 tests/unit/nft-test.h create mode 100644 tests/unit/test-libnftables-static.c create mode 100644 tests/unit/test-libnftables.c create mode 100755 tools/test-runner.sh