> On Jan 23, 2026, at 06:50, Stefan Koch <[email protected]> wrote: > > Without the call to g_test_init, the check in test-qofinstance.cpp > fails on: EXPECT_STREQ( error_message, "[qof_commit_edit()] unbalanced > call - resetting (was -2)" ). The error message is set in the local > fatal_handler which is set in the test with > g_test_log_set_fatal_handler.
That’s because without g_test_init it’s not fatal. > > The same test also does the g_log_set_handler. to the > test_checked_handler method which is in unittest-support.c. It sets > this up to print out the error, but ignore it (if it does NOT match > the way that test_error_struct is setup). But the way it is setup > does not match, because it is setup with level of critical or fatal, > which does not match critical. Again, because the log_level set > > Conclusion: The g_test_log_set_fatal_handler only does something when > g_test_init is called. Conversely, g_log_set_handler only does > something when g_test_init is not called. So, the test code should > never use both, and only one or the other depending on if it is a > google test or glib test. Not a sound conclusion. They both set handlers regardless of whether g_test_init is called. g_test_init changes when G_LOG_LEVEL_FATAL is set on a message.and that affects the operation of the handlers: the fatal handler doesn’t get called because without g_test_init very few (maybe no) GnuCash log messages are fatal, and the log level seen by the regular handler includes G_LOG_LEVEL_FATAL in fewer (maybe no) cases so test_error_struct’s log level needs to change to not include G_LOG_LEVEL_FATAL. Or you could duplicate the code at https://gitlab.gnome.org/GNOME/glib/-/blame/main/glib/gtestutils.c?ref_type=heads#L1712 to make more messages fatal. I don’t think that’s a good idea, it’s better to test with an environment as close as possible to the one that exists when the program is in normal use. > > To fix the code for the conversion to google test I am doing: > 1. Remove the g_test_init. (per recomendation) > 2. Remove the local fatal_handler, and error_message. (It had been > referenced in one other test, but not used.) > 3. Remove the check from the (now unused) error_message, and add the > check for number of hits to the test_error_struct. > 4. Fix the log level to just be the expected G_LOG_LEVEL_CRITICAL > (removing the or fatal.) > > This now passes and does the same checks as before. That’s the right thing to have done even though it seems to have been based on a mistaken understanding. > > > > When I debug, I like to use a 3 step method: > 1. Fix the issue (which is the above) Surely there’s an unstated step 0: Identify and diagnose the problem. It’s hard to fix something without doing that. > 2. Now that I know this was a problem, find and fix all similar > problems. (Below.) > a) Looking for other loglevel = with combined levels --> I have > found many. At least some of which are part of glibtest, > possibly all, possibly only in libgnucash/engine. > b) Looking for g_log_set_handler I found many, but suspect ones are > in libgnucash/engine. > c) There are multiple places that call both > g_test_log_set_fatal_handler and > g_test_log_set_fatal_handler. Maybe my interpretation above is > wrong, but I tested in the one case above. If I am right this > code is extra in one way or another. > 3. Think about what went wrong and keep it for getting this far. > a) g_log_set_handler should assert/fail if it is called after > g_test_init. This is difficult since we don't want to change > glib. The other solution is to replace g_log_set_handler with a > macro that asserts on the check for g_test_init having been > called, and then calls g_log_set_handler. > b) g_test_log_set_fatal_handler should assert if it is called > without calling g_test_init first. > > > I'm not sure how to proceed with steps 2 and 3. Now that you know what’s actually happening perhaps you can figure this out on your own. The log checking in the glib-test based tests is the way it is because of the way that g_test_init changes the behavior of the glib logging system. Porting the tests to google test leaves the logging system unchanged, so you can just remove most, perhaps all, of the fatal handlers and just use the handlers in unittest-support. You’ll need to adjust the log levels passed to those handlers and depending on what messages are checked you might need to switch from a single message checker to a list checker. You can continue to work on one file-under-test at a time. Remember that the objective here is for you to learn GnuCash’s code base and to accomplish that you'll also need to learn how Glib, GObject, and Gtk work, so read their documentation and code along with GnuCash’s as you proceed. Regards, John Ralls > > Both 3a and 3b would only affect test code, so this may be a good > idea. But, this will inject a bunch of errors into the tests. Those > would have to be fixed as part of this commit. Those fixed would > remove code that is closer for google test instead of closer to glib > test. > > Maybe my approach should be as follows: > 1. convert all existing test_qof (glib) to google test without adding > more coverage. > 2. convert all the test-engine tests to google without adding more > coverage. > 3. (Possibly) change the engine tests that use do_test mechanism tests > to google test without adding more coverage. > 4. The remaining glib tests are outside of engine. > import-export/aqb/test/test-aqb > import-export/test/test-import-pending-matches > register/ledger-core/test/test-split-register > backend/dbi/test/test-backend-dbi backend/sql/test/test-sqlbe > core-utils/test/test-gnc-glib-utils > > I'm not sure if converting these, or fixing these for step 3a, 3b > above, or ignoring this is the right step. > 5. Adding more coverage to engine. > > > On Thu, Jan 22, 2026 at 7:46 PM John Ralls <[email protected] > <mailto:[email protected]>> wrote: >> >> >> > On Jan 22, 2026, at 5:44 AM, Stefan Koch <[email protected] >> > <mailto:[email protected]>> wrote: >> > >> > I think the g_test_init is necessary mainly for the >> > g_test_log_set_fatal_handler and/or the g_log_set_handler. So, I think I >> > will need it even after I replace the glib random numbers. >> > >> >> Stefan, >> >> No, they’re both implemented in glib/gmessages.c with no reference to the >> test system. In fact you might not need to set the fatal handler if you >> don’t call g_test_init: The first thing it does is set the fatal mask to >> CRITICAL, ERROR, an WARNING. Normally only CRITICAL is fatal. >> >> For an example that uses g_log_set_handler without g_test_init see >> https://github.com/Gnucash/gnucash/blob/stable/libgnucash/backend/xml/test/gtest-load-save-files.cpp >> >> Regards, >> John Ralls >> >>
_______________________________________________ gnucash-devel mailing list [email protected] https://lists.gnucash.org/mailman/listinfo/gnucash-devel
