| Message ID | cover.1722415748.git.ps@pks.im (mailing list archive) |
|---|---|
| Headers | show |
| Series | Introduce clar testing framework | expand |
Patrick Steinhardt <ps@pks.im> writes: > - The clar gives us the ability to pick which tests to run via command > line parameters, which I personally like more than picking the > specific binary to run. One thing I am very unhappy about the current t/unit-tests/ is that the GIT_SKIP_TESTS mechanism is not effective at all. If we can wrap clar's test selection syntax inside t/Makefile to work with GIT_SKIP_TESTS (or its superset equivalent), that would be a great plus. > - The clar replaces some test assertions that we already have. They > feel a bit more mature, but overall there aren't all that many > assertions available. If we wanted to pick it up, then we'd likely > have to add some more wrappers. That is a slight bummer, as importing an externally developed one is with the hope that we won't have to enhance or maintain it, but we'll see how much burden it will be. > - The clar uses longjmp instead of manually having to `return` from > functions in case there was an assertion failure. This is easier to > work with in my opinion. > > Also, note that I only tested this on my Linux machine. I have no clue > whether this works as-is on Windows, but I do know that libgit2 tests > run on Linux, macOS and Windows. So it should work in theory, it's just > a matter of polishing this series. > > I'm happy to hear your thoughts on this, even if it ultimately ends up > being shot down. Thanks for getting the ball going. Let's see how fast and far it rolls.
On Wednesday, July 31, 2024 11:51 AM, Junio C Hamano wrote: >Patrick Steinhardt <ps@pks.im> writes: > >> - The clar gives us the ability to pick which tests to run via command >> line parameters, which I personally like more than picking the >> specific binary to run. > >One thing I am very unhappy about the current t/unit-tests/ is that the >GIT_SKIP_TESTS mechanism is not effective at all. If we can wrap clar's test selection >syntax inside t/Makefile to work with GIT_SKIP_TESTS (or its superset equivalent), >that would be a great plus. > >> - The clar replaces some test assertions that we already have. They >> feel a bit more mature, but overall there aren't all that many >> assertions available. If we wanted to pick it up, then we'd likely >> have to add some more wrappers. > >That is a slight bummer, as importing an externally developed one is with the hope >that we won't have to enhance or maintain it, but we'll see how much burden it will >be. > >> - The clar uses longjmp instead of manually having to `return` from >> functions in case there was an assertion failure. This is easier to >> work with in my opinion. >> >> Also, note that I only tested this on my Linux machine. I have no clue >> whether this works as-is on Windows, but I do know that libgit2 tests >> run on Linux, macOS and Windows. So it should work in theory, it's >> just a matter of polishing this series. >> >> I'm happy to hear your thoughts on this, even if it ultimately ends up >> being shot down. > >Thanks for getting the ball going. Let's see how fast and far it rolls. I'm sorry for being so behind the curve... what is clar and where does it run?
<rsbecker@nexbridge.com> writes:
> I'm sorry for being so behind the curve... what is clar and where does it run?
We have t/unit-test/test-lib.[ch] that are our home-grown unit test
framework. A handful of tests have been written to use it, when you
say "make test", or "(cd t && make)", unit tests binaries linked
with the home-grown unit test framework run.
clar is a _potential_ replacement for our home-grown framework,
suggested here because it would be nicer if we can use off-the-shelf
component instead of having to enhance and maintain our own.
Where and how it runs does not change even after clar turns out to
be good enough for our purpose and we commit to replace our
home-grown unit test framework with it.
On 2024.07.31 11:04, Patrick Steinhardt wrote: > Hi, > > there's been some discussion around extending our unit testing framework > to avoid duplication when declaring test functions. Right now, a testing > function has to be declared and then wired up via the test's main > function, which can be a bit annoying. In the thread, René proposes an > alternative that gets rid of this duplication by using macros. And while > that does solve the issue, there were some concerns about things being > too much "magic" while at the same time not being flexible enough. > > Part of the discussion revolved around whether we maybe want to have a > proper unit testing framework in our codebase instead of reinventing the > wheel. As I quite liked the "clar" [2] testing framework from back when > I was still developing libgit2 I proposed it as a possible alternative. > This patch series wires up the clar framework as a proof of concept and > converts the strvec test suite to use it. > > The magic to avoid the above code duplication is quite self-contained in > a "generate.py" file. This script extracts function declarations from > all unit test suites and then writes those into a "clar.suite" header > file. All that one needs to do is thus to declare a function with a > specific name "test_<suite>__<name>" and then everything else gets wired > up automatically. > > Whether this is better than the solution proposed by René is probably a > matter of taste. While I'm not a huge fan of the macro-based solution, > I don't want to veto it either (not that I'd have that power anyway). So > please, you should rather read this as a proof of concept to see how > alternatives could look like such that we have a better picture of where > we want to end up. > > Some random thoughts: > > - The mandated Python dependency is suboptimal in my opinion. > Rewriting the script in e.g. Perl should be easy enough though, it's > no rocket science. > > - I prefer that the proposed solution results in a single binary as > compared to one binary per test system. Does clar allow running test functions in parallel? With multiple binaries, we can at least run independent tests in parallel (although right now the unit tests are fewer and so much faster than the shell tests that it's hardly noticeable). > - The clar gives us the ability to pick which tests to run via command > line parameters, which I personally like more than picking the > specific binary to run. Yes this is a nice improvement. > - The clar replaces some test assertions that we already have. They > feel a bit more mature, but overall there aren't all that many > assertions available. If we wanted to pick it up, then we'd likely > have to add some more wrappers. > > - The clar uses longjmp instead of manually having to `return` from > functions in case there was an assertion failure. This is easier to > work with in my opinion. > > Also, note that I only tested this on my Linux machine. I have no clue > whether this works as-is on Windows, but I do know that libgit2 tests > run on Linux, macOS and Windows. So it should work in theory, it's just > a matter of polishing this series. > > I'm happy to hear your thoughts on this, even if it ultimately ends up > being shot down. As part of the original unit-test series, I wrote a comparison between different frameworks: Documentation/technical/unit-tests.txt, poorly rendered at [1]. Could you add a row to the table evaluating clar on the individual points there? [1] https://git-scm.com/docs/unit-tests#framework-selection > Patrick > > [1]: <85b6b8a9-ee5f-42ab-bcbc-49976b30ef33@web.de> > [2]: https://github.com/clar-test/clar > > Patrick Steinhardt (3): > t: import the clar unit testing framework > Makefile: wire up the clar unit testing framework > t/unit-tests: convert strvec tests to use clar > > .gitignore | 1 + > Makefile | 36 +- > t/Makefile | 1 + > t/unit-tests/.gitignore | 3 + > t/unit-tests/clar/.github/workflows/ci.yml | 23 + > t/unit-tests/clar/COPYING | 15 + > t/unit-tests/clar/README.md | 329 ++++++++ > t/unit-tests/clar/clar.c | 842 +++++++++++++++++++++ > t/unit-tests/clar/clar.h | 173 +++++ > t/unit-tests/clar/clar/fixtures.h | 50 ++ > t/unit-tests/clar/clar/fs.h | 522 +++++++++++++ > t/unit-tests/clar/clar/print.h | 211 ++++++ > t/unit-tests/clar/clar/sandbox.h | 154 ++++ > t/unit-tests/clar/clar/summary.h | 143 ++++ > t/unit-tests/clar/generate.py | 267 +++++++ > t/unit-tests/clar/test/.gitignore | 5 + > t/unit-tests/clar/test/Makefile | 39 + > t/unit-tests/clar/test/clar_test.h | 16 + > t/unit-tests/clar/test/main.c | 40 + > t/unit-tests/clar/test/main.c.sample | 27 + > t/unit-tests/clar/test/resources/test/file | 1 + > t/unit-tests/clar/test/sample.c | 84 ++ > t/unit-tests/{t-strvec.c => strvec.c} | 124 ++- > t/unit-tests/unit-test.c | 16 + > t/unit-tests/unit-test.h | 3 + > 25 files changed, 3041 insertions(+), 84 deletions(-) > create mode 100644 t/unit-tests/clar/.github/workflows/ci.yml > create mode 100644 t/unit-tests/clar/COPYING > create mode 100644 t/unit-tests/clar/README.md > create mode 100644 t/unit-tests/clar/clar.c > create mode 100644 t/unit-tests/clar/clar.h > create mode 100644 t/unit-tests/clar/clar/fixtures.h > create mode 100644 t/unit-tests/clar/clar/fs.h > create mode 100644 t/unit-tests/clar/clar/print.h > create mode 100644 t/unit-tests/clar/clar/sandbox.h > create mode 100644 t/unit-tests/clar/clar/summary.h > create mode 100755 t/unit-tests/clar/generate.py > create mode 100644 t/unit-tests/clar/test/.gitignore > create mode 100644 t/unit-tests/clar/test/Makefile > create mode 100644 t/unit-tests/clar/test/clar_test.h > create mode 100644 t/unit-tests/clar/test/main.c > create mode 100644 t/unit-tests/clar/test/main.c.sample > create mode 100644 t/unit-tests/clar/test/resources/test/file > create mode 100644 t/unit-tests/clar/test/sample.c > rename t/unit-tests/{t-strvec.c => strvec.c} (54%) > create mode 100644 t/unit-tests/unit-test.c > create mode 100644 t/unit-tests/unit-test.h > > -- > 2.46.0.dirty >
On Wednesday, July 31, 2024 12:53 PM, Junio C Hamano wrote: ><rsbecker@nexbridge.com> writes: > >> I'm sorry for being so behind the curve... what is clar and where does it run? > >We have t/unit-test/test-lib.[ch] that are our home-grown unit test framework. A >handful of tests have been written to use it, when you say "make test", or "(cd t && >make)", unit tests binaries linked with the home-grown unit test framework run. > >clar is a _potential_ replacement for our home-grown framework, suggested here >because it would be nicer if we can use off-the-shelf component instead of having >to enhance and maintain our own. > >Where and how it runs does not change even after clar turns out to be good enough >for our purpose and we commit to replace our home-grown unit test framework >with it. Well... I would like to be able to see whether this can be built/used on NonStop just so I can stay ahead of the curve or be far enough in advance of it to request any required fixes to make it work on platform.
On Wednesday, July 31, 2024 4:25 PM, I wrote: >On Wednesday, July 31, 2024 12:53 PM, Junio C Hamano wrote: >><rsbecker@nexbridge.com> writes: >> >>> I'm sorry for being so behind the curve... what is clar and where does it run? >> >>We have t/unit-test/test-lib.[ch] that are our home-grown unit test >>framework. A handful of tests have been written to use it, when you >>say "make test", or "(cd t && make)", unit tests binaries linked with the home- >grown unit test framework run. >> >>clar is a _potential_ replacement for our home-grown framework, >>suggested here because it would be nicer if we can use off-the-shelf >>component instead of having to enhance and maintain our own. >> >>Where and how it runs does not change even after clar turns out to be >>good enough for our purpose and we commit to replace our home-grown >>unit test framework with it. > >Well... I would like to be able to see whether this can be built/used on NonStop just >so I can stay ahead of the curve or be far enough in advance of it to request any >required fixes to make it work on platform. After checking out clar, I would say it has potential, but needs to be slightly more portable to depend on more than gcc CFLAGS (specifically -Wall). I will ask that of that team. It does look like it might be possible, but it should integrate with the config.mak.uname settings, so c99 or c11 (or soon c17) could be used with other CFLAGS from that file so maintainers only have to worry about one location. --Randall
On Wed, Jul 31, 2024 at 11:33:55AM -0700, Josh Steadmon wrote: > On 2024.07.31 11:04, Patrick Steinhardt wrote: > > - I prefer that the proposed solution results in a single binary as > > compared to one binary per test system. > > Does clar allow running test functions in parallel? With multiple > binaries, we can at least run independent tests in parallel (although > right now the unit tests are fewer and so much faster than the shell > tests that it's hardly noticeable). Ah, that's something I didn't think of. clar does not support running tests in parallel. As you say, I guess for now that is fine and I'd claim that it likely is faster to just run all tests sequentially with a single binary if you also include build times. If that claim isn't true, or if we eventually grow a huge body of tests, then we should likely revert to having separate binaries. > As part of the original unit-test series, I wrote a comparison between > different frameworks: Documentation/technical/unit-tests.txt, poorly > rendered at [1]. Could you add a row to the table evaluating clar on the > individual points there? > > [1] https://git-scm.com/docs/unit-tests#framework-selection Ah, I wasn't aware of this document. I can update it depending on how the discussion goes overall :) Patrick
On Wed, Jul 31, 2024 at 08:51:00AM -0700, Junio C Hamano wrote: > Patrick Steinhardt <ps@pks.im> writes: > > > - The clar gives us the ability to pick which tests to run via command > > line parameters, which I personally like more than picking the > > specific binary to run. > > One thing I am very unhappy about the current t/unit-tests/ is that > the GIT_SKIP_TESTS mechanism is not effective at all. If we can > wrap clar's test selection syntax inside t/Makefile to work with > GIT_SKIP_TESTS (or its superset equivalent), that would be a great > plus. Yeah, I guess that shouldn't be too hard. > > - The clar replaces some test assertions that we already have. They > > feel a bit more mature, but overall there aren't all that many > > assertions available. If we wanted to pick it up, then we'd likely > > have to add some more wrappers. > > That is a slight bummer, as importing an externally developed one is > with the hope that we won't have to enhance or maintain it, but > we'll see how much burden it will be. The clar exposes generic helpers like `cl_assert`, but also comparison functions like `cl_assert_equal_$t` for pointers, integers and strings. If anything is missing, it also exposes the building blocks to add project-specific assertions so that it would be easy to add for example `cl_assert_equal_oid`. One thing I have been missing is non-equality comparisons like `cl_assert_lt_i`/`cl_assert_gt_i`. But adding that to the clar itself and upstreaming it should be easy enough, also because we know the people maintaining it. Other than that I think it's mostly fine and should serve as a good baseline. Patrick
On Wed, Jul 31, 2024 at 04:37:37PM -0400, rsbecker@nexbridge.com wrote: > On Wednesday, July 31, 2024 4:25 PM, I wrote: > >On Wednesday, July 31, 2024 12:53 PM, Junio C Hamano wrote: > >><rsbecker@nexbridge.com> writes: > >> > >>> I'm sorry for being so behind the curve... what is clar and where does it run? > >> > >>We have t/unit-test/test-lib.[ch] that are our home-grown unit test > >>framework. A handful of tests have been written to use it, when you > >>say "make test", or "(cd t && make)", unit tests binaries linked with the home- > >grown unit test framework run. > >> > >>clar is a _potential_ replacement for our home-grown framework, > >>suggested here because it would be nicer if we can use off-the-shelf > >>component instead of having to enhance and maintain our own. > >> > >>Where and how it runs does not change even after clar turns out to be > >>good enough for our purpose and we commit to replace our home-grown > >>unit test framework with it. > > > >Well... I would like to be able to see whether this can be built/used on NonStop just > >so I can stay ahead of the curve or be far enough in advance of it to request any > >required fixes to make it work on platform. > > After checking out clar, I would say it has potential, but needs to be slightly more > portable to depend on more than gcc CFLAGS (specifically -Wall). I will ask that > of that team. It does look like it might be possible, but it should integrate with the > config.mak.uname settings, so c99 or c11 (or soon c17) could be used with other > CFLAGS from that file so maintainers only have to worry about one location. I'm not sure I follow what you're saying. We don't use the Makefile of clar at all but integrate it into our own build system, so honoring config.mak.uname should come for free, I think. In any case, if there are C constructs in clar that do not work right now, then I'd be happy to address those issues. The clar is overall quite small, so it shouldn't be all that involved. Patrick
Hi, there's been some discussion around extending our unit testing framework to avoid duplication when declaring test functions. Right now, a testing function has to be declared and then wired up via the test's main function, which can be a bit annoying. In the thread, René proposes an alternative that gets rid of this duplication by using macros. And while that does solve the issue, there were some concerns about things being too much "magic" while at the same time not being flexible enough. Part of the discussion revolved around whether we maybe want to have a proper unit testing framework in our codebase instead of reinventing the wheel. As I quite liked the "clar" [2] testing framework from back when I was still developing libgit2 I proposed it as a possible alternative. This patch series wires up the clar framework as a proof of concept and converts the strvec test suite to use it. The magic to avoid the above code duplication is quite self-contained in a "generate.py" file. This script extracts function declarations from all unit test suites and then writes those into a "clar.suite" header file. All that one needs to do is thus to declare a function with a specific name "test_<suite>__<name>" and then everything else gets wired up automatically. Whether this is better than the solution proposed by René is probably a matter of taste. While I'm not a huge fan of the macro-based solution, I don't want to veto it either (not that I'd have that power anyway). So please, you should rather read this as a proof of concept to see how alternatives could look like such that we have a better picture of where we want to end up. Some random thoughts: - The mandated Python dependency is suboptimal in my opinion. Rewriting the script in e.g. Perl should be easy enough though, it's no rocket science. - I prefer that the proposed solution results in a single binary as compared to one binary per test system. - The clar gives us the ability to pick which tests to run via command line parameters, which I personally like more than picking the specific binary to run. - The clar replaces some test assertions that we already have. They feel a bit more mature, but overall there aren't all that many assertions available. If we wanted to pick it up, then we'd likely have to add some more wrappers. - The clar uses longjmp instead of manually having to `return` from functions in case there was an assertion failure. This is easier to work with in my opinion. Also, note that I only tested this on my Linux machine. I have no clue whether this works as-is on Windows, but I do know that libgit2 tests run on Linux, macOS and Windows. So it should work in theory, it's just a matter of polishing this series. I'm happy to hear your thoughts on this, even if it ultimately ends up being shot down. Patrick [1]: <85b6b8a9-ee5f-42ab-bcbc-49976b30ef33@web.de> [2]: https://github.com/clar-test/clar Patrick Steinhardt (3): t: import the clar unit testing framework Makefile: wire up the clar unit testing framework t/unit-tests: convert strvec tests to use clar .gitignore | 1 + Makefile | 36 +- t/Makefile | 1 + t/unit-tests/.gitignore | 3 + t/unit-tests/clar/.github/workflows/ci.yml | 23 + t/unit-tests/clar/COPYING | 15 + t/unit-tests/clar/README.md | 329 ++++++++ t/unit-tests/clar/clar.c | 842 +++++++++++++++++++++ t/unit-tests/clar/clar.h | 173 +++++ t/unit-tests/clar/clar/fixtures.h | 50 ++ t/unit-tests/clar/clar/fs.h | 522 +++++++++++++ t/unit-tests/clar/clar/print.h | 211 ++++++ t/unit-tests/clar/clar/sandbox.h | 154 ++++ t/unit-tests/clar/clar/summary.h | 143 ++++ t/unit-tests/clar/generate.py | 267 +++++++ t/unit-tests/clar/test/.gitignore | 5 + t/unit-tests/clar/test/Makefile | 39 + t/unit-tests/clar/test/clar_test.h | 16 + t/unit-tests/clar/test/main.c | 40 + t/unit-tests/clar/test/main.c.sample | 27 + t/unit-tests/clar/test/resources/test/file | 1 + t/unit-tests/clar/test/sample.c | 84 ++ t/unit-tests/{t-strvec.c => strvec.c} | 124 ++- t/unit-tests/unit-test.c | 16 + t/unit-tests/unit-test.h | 3 + 25 files changed, 3041 insertions(+), 84 deletions(-) create mode 100644 t/unit-tests/clar/.github/workflows/ci.yml create mode 100644 t/unit-tests/clar/COPYING create mode 100644 t/unit-tests/clar/README.md create mode 100644 t/unit-tests/clar/clar.c create mode 100644 t/unit-tests/clar/clar.h create mode 100644 t/unit-tests/clar/clar/fixtures.h create mode 100644 t/unit-tests/clar/clar/fs.h create mode 100644 t/unit-tests/clar/clar/print.h create mode 100644 t/unit-tests/clar/clar/sandbox.h create mode 100644 t/unit-tests/clar/clar/summary.h create mode 100755 t/unit-tests/clar/generate.py create mode 100644 t/unit-tests/clar/test/.gitignore create mode 100644 t/unit-tests/clar/test/Makefile create mode 100644 t/unit-tests/clar/test/clar_test.h create mode 100644 t/unit-tests/clar/test/main.c create mode 100644 t/unit-tests/clar/test/main.c.sample create mode 100644 t/unit-tests/clar/test/resources/test/file create mode 100644 t/unit-tests/clar/test/sample.c rename t/unit-tests/{t-strvec.c => strvec.c} (54%) create mode 100644 t/unit-tests/unit-test.c create mode 100644 t/unit-tests/unit-test.h