Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] Form: Add _warning attribute to check its value in tests #107272

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

Conversation

rousseldenis
Copy link
Contributor


I confirm I have signed the CLA and read the PR guidelines at www.odoo.com/submit-pr

@robodoo
Copy link
Contributor

robodoo commented Dec 6, 2022

@rousseldenis
Copy link
Contributor Author

@rco-odoo

@C3POdoo C3POdoo requested review from a team December 6, 2022 10:01
@Julien00859
Copy link
Member

Julien00859 commented Dec 6, 2022

wouldn't assertLogs work?

@Xavier-Do
Copy link
Contributor

You could also patch the onchange method to intercept the result if needed.

@rousseldenis
Copy link
Contributor Author

You could also patch the onchange method to intercept the result if needed.

Integrated helper should be great (as don't need to patch each time).

But I didn't know the assertLogs().

The working command looks like this:

image

IMHO, the logger attribute is quite tricky for normal developers. Don't you think ?

@xmo-odoo
Copy link
Collaborator

xmo-odoo commented Dec 6, 2022

image

You don't need to use getChild when the entire logger name is hardcoded. Furthermore assertLogs can take a logger or a logger name.

So the actual code is

with self.assertLogs("odoo.tests.common.onchange", level=logging.WARNING):
    ...

which is

  1. standard code you can find in the official Python documentation
  2. applicable to any situation where you want to check logging

IMHO, the logger attribute is quite tricky for normal developers. Don't you think ?

More tricky than a magical attribute which may or may not be present and can't be retrieved normally?

@rousseldenis
Copy link
Contributor Author

image

You don't need to use getChild when the entire logger name is hardcoded. Furthermore assertLogs can take a logger or a logger name.

So the actual code is

with self.assertLogs("odoo.tests.common.onchange", level=logging.WARNING):
    ...

which is

  1. standard code you can find in the official Python documentation
  2. applicable to any situation where you want to check logging

IMHO, the logger attribute is quite tricky for normal developers. Don't you think ?

Great!

It was a proposal to enhance dev experience in test writing.

Especially in this case.

More tricky than a magical attribute which may or may not be present and can't be retrieved normally?

We could maybe initialize the attribute as void to make it persistent and have an helper method to retrieve it.

My two cents.

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

Successfully merging this pull request may close these issues.

None yet

5 participants