Haha, I guess we have a gazillion tests that use "reference logs" (which contain what-triggered-off-what info) but never thought to make a test that generates one 😬

Adding @matthewrmshin as reviewer - he is the author of the relevant code block.

An over-the-top 2to3 change?

@kinow @matthewrmshin do you agree with my comment:

Shouldn't we just open the reference log file in text mode??

If so let's just do that, no? (The only reason I'm a bit hesitant is I'm not fully up to speed yet with Python 3 string encoding ... but I expect you guys are).

Now that I am awake on Monday... The problem with text mode file in Python <3.7 is that it is susceptible to locale setting. The horrible LANG=C simply makes everything go haywire. The problem does appear to be fixed in 3.7 with PEP538 implemented.

Does that matter in this particular context though? (even before 3.7) (we are already opening the main log in text mode...)

It is probably OK in this context.

And now that I am really thinking (Monday mid morning - best thinking time)...

Instead of parsing the log, and filtering it, we should just add a custom log handler to the logger when we are in --reference-log mode - which should filter log messages and write the relevant stuffs to the reference.log file. Probably a follow-on when we revisit logging again.

Instead of parsing the log, and filtering it, we should just add a custom log handler to the logger when we are in --reference-log mode - which should filter log messages and write the relevant stuffs to the reference.log file. Probably a follow-on when we revisit logging again.

Yes I was wondering about that myself. The current method isn't very elegant, but I suppose it doesn't really matter because generating a reference log is a rare thing to do, and doesn't affect users.

@hjoliver , I hadn't looked around the code when I first suggested the fix. After the comments here, got a cup of coffee and read what the reference log was doing, and @matthewrmshin 's comment about using a logging.Handler seemed like a good idea.

Every logging.Handler is also a logging.Filterer (i.e. extends it). So we can add filterers with #addFilter, which is pretty much what we need with the reference log... as far as I could tell. And it seemed easier than fixing the existing code for binary/text.

I tried it locally with ol' suite number five, and got similar output (only the timestamps differ, obviously). So pushed the changes here to see if Travis-CI passes all tests OK.

Only issue I actually had was that we cannot initialize the new ReferenceLogHandler with the other logging.Handler's in Scheduler#_setup_suite_logger, because _setup_suite_logger is called before the configure method. And ReferenceLogHandler depends on the configuration being loaded for the reference log output directory.

So the ReferenceLogHandler is initialized in the same part it was already checking for the option to generate reference log (i.e. it should kill 1 if statement, making Codacy a bit happier).

Hope that makes sense?