Decide what to do with logger calls in test environments

[raquelalegre]

@giordano suggests that when we run tests and call functions that log something with the logger, those messages shouldn't go in the same logger as the production ones. He suggests one solution could be functions that use the logger should be passed the logger, defaulting to the global logger so it's not necessary to always pass an instance of the logger when calling the function.

@raquel-ucl suggests do one of these three:

1. Leave as is, since it's unclear why logging in testing mode by us developers would need to be different than non-test mode development testing. Users will never run tests, and we'll never run in production.
2. Configure the logger (if possible, I haven’t checked! You can do it in log4j and others though) to look into:
   • the trace, so if it’s called from the tests classes it uses a different file/logger; or
   • the environment, so we have a production and a test environment, and the logger acts accordingly
3. Pass the logger to the constructors, not the functions (which can be problematic for future development, e.g. functions which are not logging anything but are in turn calling functions that do need a logger). Ideally we can use dependency injection but that is less simple to implement.

We need to have a conversation about this because something might have been lost in Slack comms!

[raquelalegre]

We considered having 2 separate logs: one containing the global user log, and one containing the logs produced by the tests. However this makes it very complicated to assert the correctness of the log content in the tests.

We have decided that:

• Testing that will also include asserting the log contents are correct, will keep on using temporary logs as it is now.
• Testing that produces log that might be useful in the future, like the communication with the server, we will kept in a secondary test log.

If the secondary test log can't be done smoothly, worst case scenario would be to use the global user log. We'll try to avoid this, though.

[giordano]

For what is worth, I'm happy with the current situation. What I was mostly concerned about was that logger tests at

nisaba/src/test/suite/logger.test.ts

Lines 26 to 33 in 28ea735

	const text = fs.readFileSync(tmpobj.name).toString().split(os.EOL);
	for (let i = 0; i < text.length; i++) {
	// Skip empty lines
	if (text[i]) {
	const message = new RegExp('\\[nisaba\\] ' + messages[i][0] + ': ' + messages[i][1]);
	assert.match(text[i], message);
	}
	}

should be easy to do without fiddling too much to find the beginning and the end of the test log, and this is done in a temporary file.