make check fails

[John Ralls]

On Jun 13, 2013, at 10:16 AM, Christian Stimming <christian at 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