On Jun 13, 2013, at 10:16 AM, Christian Stimming <christ...@cstimming.de> wrote:
> Am Donnerstag, 13. Juni 2013, 10:05:49 schrieb John Ralls: >> You wrote in the commit message: >> "Sigh. It turns out the utest-Split.c relies on the "warning" log level >> in order to check for specific code paths. This sucks. The log level >> "warning" should please be reserved for things that are actual warnings, >> not for code path checks that are used in the unittests." >> >> That's not correct. utest-Split may rely on the presence of the message to >> check the code path, and in order to do so needs to match the loglevel, >> logdomain, and message string. If you change one of those in the tested >> function, you just change it to match in the test and it should work. In >> this case you want to change the "foo differ" warnings to infos. No big >> deal, just change the loglevel in the tests to match. > > Hi John, > > the problem is that it's not sufficient to "just change it to match the > test". > I saw in utest-Split.c lines 424-428 how a set of loglevels was selected for > various check counters. However, adding the G_LOG_LEVEL_INFO to the set of > loglevels in line 424 doesn't work because other tests rely on exactly the > chosen set of loglevels that are in use now. Changing the set of log levels > solely for the checkC counter doesn't work either, because even this one > relies on exactly only this set log level in some other code path tests. > That's when I gave up and summarized this experienced in the quoted commit > message. Loglevel isn't a mask: It has to match exactly. It's only a variable to save typing G_LOG_LEVEL_WARNING | G_LOG_FLAG_FATAL everywhere and to shorten line lengths. One could use guint loglevel_info = G_LOG_LEVEL_INFO //g_tester doesn't make info messages fatal or just use G_LOG_LEVEL_INFO directly in the two or three places it's needed. >> Now, ideally unit tests would only be checking the message to make sure >> that it's emitted: That's a side-effect of the function, and a unit test >> should test all of the function's effects. Unfortunately most of the engine >> consists of over-long functions with multiple code paths whose behavior is >> difficult to test correctly coupled with excessive class interdependencies >> . In order to be able to refactor them for better testability I have to >> first get them tested somehow so that I'll know when I break something in >> the refactor. > > I certainly agree the overly-long functions make unittesting even harder. > Maybe I'm just not understanding the unittesting that includes checking for > log output. In that case I'd appreciate if you change the PWARN into PINFO > and > correctly adapt the unittest. Thanks! Unit tests should test a function's every behavior, and emitting messages is a behavior. What's hard to understand about that? I'll take care of it, but I can't promise that it will be in time for 2.5.3. BTW, Split.c needs the same treatment. There are probably other places as well. Regards, John Ralls _______________________________________________ gnucash-devel mailing list gnucash-devel@gnucash.org https://lists.gnucash.org/mailman/listinfo/gnucash-devel
