Skip to content
This repository has been archived by the owner on May 16, 2024. It is now read-only.

[FIX] test_server: Add ERROR log level #352

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lasley
Copy link
Contributor

@lasley lasley commented Sep 15, 2016

This is a questionable fix for #348 - not exactly sure how to test though 😑

  • Also listen for ERROR log level on versions >7

* Also listen for ERROR log level on versions >7
@moylop260
Copy link
Contributor

You need increase the number of EXPECTED ERROR for this first build from .travis.yml file in order to consider the new type of error.

@pedrobaeza
Copy link
Member

The problem with this is that sometimes we test that there are errors and that the errors raise, like this case: https://travis-ci.org/OCA/server-tools/jobs/166895188#L1045. Maybe tweaking EXPECTED_ERRORS can work for these cases.

@lasley
Copy link
Contributor Author

lasley commented Oct 12, 2016

Why are we not catching the errors with self.assertRaises()? Or are these simply logging.error calls?

@pedrobaeza
Copy link
Member

They are caught that way, but this doesn't prevent the ERROR message in the log.

@lasley
Copy link
Contributor Author

lasley commented Oct 12, 2016

That seems to only be in instances where a logger is specifically outputting an error message via logging.error. The following example generates no log:

>>> import unittest
>>> 
>>> def error():
...     raise Exception('Derp')
... 
>>> class MyTest(unittest.TestCase):
...     def test(self):
...         with self.assertRaises(Exception):
...             error()
... 
>>> unittest.main()
.
----------------------------------------------------------------------
Ran 1 test in 0.000s

OK

Regardless, that means we're on fairly thin ice with this solution in terms of scalability IMO.

Is there ever an instance when log output is actually relevant to a test? It seems weird to me that we would test a log output, so what if we were to just raise the Odoo log level to CRITICAL instead of ERROR? This shouldn't affect tracebacks from real errors, it will only suppress superfluous logging calls meant to be read by a user.

@pedrobaeza
Copy link
Member

The problem is that you don't have control over that message that is spit out by Odoo. You only control the exception with assertRaises. Anyway, we can add the expected errors variable to control this. @moylop260, what do you think?

@moylop260
Copy link
Contributor

moylop260 commented Oct 13, 2016

The case of example using with self.assertRaises(Exception) still show log.error from odoo orm.

Example:

import unittest
import odoo

class MyTest(unittest.TestCase):
     def test(self):
       with self.assertRaises(Exception):  # Mute a raise
             odoo.method()  # This method have a log.error('msg') and the log show it

We can use mute_logger in order to avoid show it.

import unittest
import odoo

class MyTest(unittest.TestCase):
     @odoo.mute_logger('logger.my_logger')  # Mute logger to avoid show the error in the log
     def test(self):
       with self.assertRaises(Exception):   # Mute a raise
             odoo.method()

Example of this case

For the sample modules of MQT we need the test case to validate that the errors are processed correctly, then we shouldn't use a mute.

We are using a counter of errors.
Then if you increase the counter (for CRITICAL in this case) we need increase the expected value in order to test the case and have a CI green.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants