oracc / nisaba

GNU General Public License v3.0
3 stars 0 forks source link

Decide what to do with logger calls in test environments #43

Open raquelalegre opened 3 years ago

raquelalegre commented 3 years ago

@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 commented 3 years ago

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:

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 commented 3 years ago

For what is worth, I'm happy with the current situation. What I was mostly concerned about was that logger tests at https://github.com/oracc/nisaba/blob/28ea7351fc65dea71ea6b0d2ea905154bc17f328/src/test/suite/logger.test.ts#L26-L33 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.