# The unit tests thread **URL:** \ **Category:** Development\ **Tags:** C\ **Created:** [21 January 2021 11:22 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597 "2021-01-21T11:22:21Z")\ **Posts on this page:** 20\ **Page:** 1
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [21 January 2021 11:22 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/1 "2021-01-21T11:22:22Z")
This is the thread to discuss testing igraph’s C core: writing unit tests, improving existing tests, improving test coverage. Generally, any testing topic that’s too small to start its own thread for can go here. ---
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [21 January 2021 11:25 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/2 "2021-01-21T11:25:02Z")
@GroteGnoom It seems that there is a problem with using `assert` in unit tests: when compiling in `Release` configuration, CMake defines` NDEBUG` by default. `NDEBUG` disables `assert`: no code wrapped in `assert` will run. Eventually, we will move to a better unit testing setup, but not before the 0.9 release. For now, in any future tests you contribute, can you please use `IGRAPH_ASSERT` instead of `assert`? It works identically. There is no need to correct the old tests using `assert`—I’ll do that soon. ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [21 January 2021 17:21 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/3 "2021-01-21T17:21:26Z")
What’s the expected behavior if you provide a zero probability distribution? For example in igraph\_callaway\_traits\_game you should provide a distribution of types for each vertex. This function checks that the distribution is non-negative. But it doesn’t check that at least one value is positive. So if you provide only zeros, this distribution is non-normalizable, and it seems to me like this shouldn’t be allowed. This this situation be checked for, and return an IGRAPH\_EINVAL? > ---
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [21 January 2021 18:53 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/4 "2021-01-21T18:53:21Z")
Good catch! Yes, report an error with `IGRAPH_EINVAL`. You can check that the value of `maxcum`, computed below, is strictly positive. ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [25 January 2021 10:32 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/5 "2021-01-25T10:32:55Z")
I am working on an ERRORF macro, ([Add lastcit game unit tests by GroteGnoom · Pull Request #1625 · igraph/igraph · GitHub](https://github.com/igraph/igraph/pull/1625#issuecomment-766604852)), and I wanted to test it, but errorf always prints to stderr, and the tests can only check stdout. Can we add .err files to check stderr? ---
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [25 January 2021 11:56 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/6 "2021-01-25T11:56:09Z")
> I am working on an ERRORF macro, Thanks for working on an `ERRORF` macro, this will be a great addition! Please also add a `WARNINGF` and `FATALF` (recently a `FATALF` would have been useful when integrating the new version of Bliss). igraph did not have this feature before because of uncertain compiler support (especially MSVC). However, our minimum requirement for igraph 0.9, VS2015, does support variadic macros, so we’re good. > Can we add .err files to check stderr? I would omit this for now to keep things simple. I would not test the _text_ of error messages (as opposed to the error code) in general functions. Thus this feature would be used only for testing the error reporting function itself. That function is sufficiently simple and the risk of breakage is sufficiently low that we can verify it manually. A unit test is not needed. However, should you want to test it anyway, it is possible by setting a new error handler that prints to stdout instead of stderr. I added such a test for (the much less commonly used) `igraph_fatal` recently: [tests: fatal error handler · igraph/igraph@01dd666 · GitHub](https://github.com/igraph/igraph/commit/01dd666d1f2ec28c80d7e80c8cb8ce8d8b794a4c) ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [25 January 2021 12:12 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/7 "2021-01-25T12:12:49Z")
> [@szhorvat](#): > > his feature would be used only for testing the error reporting function itself. That function is sufficiently simple and the risk of breakage is sufficiently low that we can verify it manually. A unit test is not needed. The main thing I want to test is if it works well on all compilers, using the CI pipeline. For example, more complex variadic macro usage can be buggy: [c++ - MSVC++ variadic macro expansion - Stack Overflow](https://stackoverflow.com/questions/9183993/msvc-variadic-macro-expansion). > [@szhorvat](#): > > However, should you want to test it anyway, it is possible by setting a new error handler that prints to stdout instead of stderr. I added such a test for (the much less commonly used) `igraph_fatal` recently: [tests: fatal error handler · igraph/igraph@01dd666 · GitHub](https://github.com/igraph/igraph/commit/01dd666d1f2ec28c80d7e80c8cb8ce8d8b794a4c) Of course! That makes so much sense 🙂. ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [27 January 2021 11:09 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/8 "2021-01-27T11:09:35Z")
Should we have printf macros for things like igraph\_integer\_t? I’m now putting in %ld, but that’s only correct with certain settings. I see some places where variables are cast to long, but that can cast something from 64 bits to 32 bits. Maybe casting everything to 64 bits helps? Then we could use `PRId64`. Or we could use `%lld` and cast to `(long long)`, which seems to be 64 bits pretty much everywhere ([64-bit computing - Wikipedia](https://en.wikipedia.org/wiki/64-bit_computing#64-bit_data_models)). ---
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [28 January 2021 10:16 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/9 "2021-01-28T10:16:12Z")
Indeed, there should be a format specifier for `igraph_integer_t`. We plan to overhaul the integer types in igraph “soon” (still, this will take a few months), and the type will likely become platform-specific (i.e. 64-bit when possible, 32-bit otherwise). At that point, I was planning to add a format specifier for `igraph_integer_t`, although we have not discussed this in detail yet. This work is going to involve many big and disruptive changes, so it will happen after the next release only (due in ~1 month) Right now `igraph_integer_t` is an `int`, so `%d` works. In _tests_, I think it’s better not to rely on the fact the knowledge that it’s an `int` but to explicitly cast values to `int` (or `long` or whatever) so the code would be immune to future changes to what `igraph_integer_t` is. Example: `igraph_integer_t foo = 5; printf("Foo is %d\n", (int) foo);` In almost all tests (which deal with specific cases), we do not need very large numbers, so casting to `int` is good enough. In internal code, or user code that must work with any input (not just specific input, as in tests), of course casting to a narrow type risks losing precision, so it’s not acceptable. This is why eventually a format specifier macro will be needed. ---
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [28 January 2021 10:24 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/10 "2021-01-28T10:24:02Z")
Now I realize that you are talking about `ERRORF`, not tests. Perhaps we could add an _internal_ format specifier to go with `igraph_integer_t`. I would like to keep it non-public for the moment so that we don’t have to commit to it. @tamas @vtraag any objections to adding this? I would add it to `igraph_types.h` (even if this header is public) with a big fat warning comment that this is not for public use. Maybe name it `IGRAPH_PRId` or `IGRAPH_I_PRId`? (Renaming at a later stage should be easy, if we decide to do so.) Here’s an example of a need for it: [Add cited\_type\_game unit tests by GroteGnoom · Pull Request #1632 · igraph/igraph · GitHub](https://github.com/igraph/igraph/pull/1632/files#diff-eca3b9cc24ace7d4022c1039d0bdc013c790ee6ef2bd10bf2fc6d7d40d0ec6cfR253) Standard format specifier macros: [Fixed width integer types (since C99) - cppreference.com](https://en.cppreference.com/w/c/types/integer) ---
**Author:** ![tamas](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/tamas/32/1261_2.png) [@tamas](https://igraph.discourse.group/u/tamas)\ **Post date:** [28 January 2021 15:09 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/11 "2021-01-28T15:09:06Z")
I’m totally fine with this. Use `IGRAPH_I_PRId` to make it clear that it’s internal. Maybe we should also add an `IGRAPH_I_PRIf` for `igraph_real_t` for sake of consistency? ---
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [28 January 2021 15:44 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/12 "2021-01-28T15:44:50Z")
> [@tamas](#): > > Maybe we should also add an `IGRAPH_I_PRIf` for `igraph_real_t` for sake of consistency? As far as `printf` is concerned, there is no difference between `float` and `double` (`float` gets auto-converted to `double` when passed as a variadic function argument). So the only possible floating point type that `igraph_real_t` could be that takes something else than `%f` in `printf` is `long double`. That’s a weird type which is not commonly used, and there is no sign that people would start using it instead of `double` in the foreseeable future (in fact, quite the opposite: it’s going out of fashion). Thus, I don’t think we need this for the floating point case. ---
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [28 January 2021 16:05 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/13 "2021-01-28T16:05:51Z")
@GroteGnoom I just added a format specifier. Can you please update your PRs to use it? Additionally, please use the spelling “out-degree” with a dash instead of “out degree”. It’s not a big deal, I’ll fix any instances I find manually before merging. ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [28 January 2021 16:36 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/14 "2021-01-28T16:36:59Z")
Awesome! I’ll try it out immediately 🙂 ---
**Author:** ![szhorvat](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/szhorvat/32/3_2.png) [@szhorvat](https://igraph.discourse.group/u/szhorvat)\ **Post date:** [29 January 2021 10:01 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/15 "2021-01-29T10:01:54Z")
@GroteGnoom By what name would you like to be acknowledged? Grote Gnoom, Daniel Noom or something else? ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [29 January 2021 17:24 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/16 "2021-01-29T17:24:37Z")
Daniel Noom please 🙂 ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [4 February 2021 14:33 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/17 "2021-02-04T14:33:38Z")
I’m working on modularity\_matrix and if you put in zero edges, or your weights sum up to zero, every element of the matrix wil be either NaN or inf. Should I just leave that as it is, check for it and warn users, or check for it and give an Error? ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [8 February 2021 14:01 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/18 "2021-02-08T14:01:13Z")
Why is there a IGRAPH\_EINVVID but no IGRAPH\_EINVEID for edges? Right now if an invalid edge id is encountered sometimes an IGRAPH\_EINVVID is emitted, but sometimes an ERROR\_EINVAL. Should they maybe all be EINVAL? Or should EINVEID be added? ---
**Author:** ![tamas](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/tamas/32/1261_2.png) [@tamas](https://igraph.discourse.group/u/tamas)\ **Post date:** [8 February 2021 15:05 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/19 "2021-02-08T15:05:15Z")
I don’t know; to be honest, I did not even remember that we’ve had `IGRAPH_EINVVID` and I was mostly using `IGRAPH_EINVAL` for both cases. We could add `IGRAPH_EINVEID` for sake of completeness, but then we would be breaking the API in lots of places where we have used `IGRAPH_EINVAL` before to indicate an invalid edge ID. Sooner or later we should do it, and we are breaking lots of stuff in igraph 0.9 anyway, so I don’t have a problem with it if we do it now. But it’s also okay to postpone it to 0.10 instead. One thing that is definitely _not_ okay, though, is to use `IGRAPH_EINVVID` to indicate a problem with an edge ID. The reason is that `igraph_strerror()` converts this error code to `"Invalid vertex id"`, which is clearly misleading if the problem is with an edge ID and not a vertex ID. So, the bottom line is: - use `IGRAPH_EINVVID` for invalid vertex IDs - use `IGRAPH_EINVAL` for invalid edge IDs - later on we should probably add `IGRAPH_EINVEID` to indicate invalid edge IDs, and at the same time we should evaluate all the usages of `IGRAPH_EINVAL` to see if `IGRAPH_EINVVID` or `IGRAPH_EINVEID` would be more appropriate ---
**Author:** ![GroteGnoom](https://yyz2.discourse-cdn.com/free1/user_avatar/igraph.discourse.group/grotegnoom/32/428_2.png) [@GroteGnoom](https://igraph.discourse.group/u/GroteGnoom)\ **Post date:** [9 February 2021 10:37 UTC](https://igraph.discourse.group/t/the-unit-tests-thread/597/20 "2021-02-09T10:37:33Z")
There is a calloc here: > Which doesn’t get cleaned up after an error here: > Should the FINALLY stack handle that? I’m very confused about its usage in this file, because I see a lot of FINALLY\_CLEANs without anything being destroyed. [Next page](https://igraph.discourse.group/t/the-unit-tests-thread/597.md?page=2)