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] tests: xUnit reports, nicer STDERR output with logfile #151728

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

Conversation

yajo
Copy link
Contributor

@yajo yajo commented Jan 30, 2024

This feature allows generating automated and beautiful reports out of unit test executions.

This format is easy to integrate in common development forges:

Also, when you run tests with --logfile, now Odoo produces a succinct live test output summary to STDERR:

Grabacion.de.pantalla.desde.2024-01-25.13-35-26.webm.mp4

@moduon MT-1075


Here are backports, for those that want to start using this today:


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

This feature allows generating automated and beautiful reports out of unit test executions.

This format is easy to integrate in common development forges:

- Gitlab: https://docs.gitlab.com/ee/ci/testing/unit_test_reports.html
- Github: https://github.com/marketplace/actions/test-reporter

Also, when you run tests with `--logfile`, now Odoo produces a succinct live test output summary to STDERR.

@moduon MT-1075
@robodoo
Copy link
Contributor

robodoo commented Jan 30, 2024

Pull request status dashboard.

@C3POdoo C3POdoo requested review from a team January 30, 2024 10:27
@yajo yajo changed the title tests: xUnit reports, nicer STDERR output with logfile [IMP] tests: xUnit reports, nicer STDERR output with logfile Jan 30, 2024
Copy link
Contributor

@Xavier-Do Xavier-Do left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello and thanks for your contribution.

It looks interesting but right now I have difficulties to find a real use for odoo, since we have our own test environment (using log-db to get formatted logs as output).

This is adding complexity and maintenance to the test runner for a feature we don't use and don't test. It makes OdooTestResult inherit indirectly from TextTestResult, making it more dependant on unit test, we got some issue with that because of changes made in unittest breaking customisation we have. This is why we are trying to be more and more independent from unittest, and other libs.

I understand that it may be useful in some cases but I would prefer a less intrusive integration of this feature.

It may actually be interesting for developers if a local tool could display this result, didn't had the time to investigate. Anyway since it was mostly developed for .net it may not be ideal. As an example it looks like subTests are not well supported, and they are quite widely used in odoo. It would need some testing but I'm a bit skeptical right now. If we decide to add the tooling the output a test report in a parsable format, why should it be xUnit?

@@ -45,6 +45,7 @@ reportlab==3.6.8 ; python_version <= '3.10'
reportlab==3.6.12 ; python_version > '3.10'
requests==2.25.1 # versions < 2.25 aren't compatible w/ urllib3 1.26. Bullseye = 2.25.1. min version = 2.22.0 (Focal)
rjsmin==1.1.0
unittest-xml-reporting==3.2.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not adding requirements only useful for test in requirements.txt.

Moreover all requirements should have a debian package version equivalent since odoo should run without requirements.

Also, this is not useful for odoo developers and runbot so this should be 100% optional.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it because I needed its base class to extend it and call super(), so it handles the creation of xunit reports.

@@ -198,6 +198,9 @@ def __init__(self, fname=None):
group.add_option("--screenshots", dest="screenshots", action="store", my_default=temp_tests_dir,
metavar='DIR',
help="Screenshots will go in DIR/{db_name}/screenshots. Defaults to %s." % temp_tests_dir)
group.add_option("--test-reports", dest="test_reports", action="store", my_default=temp_tests_dir,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are trying to avoid new options as much as possible, not sure this as chances to be accepted.

self.stream.writeln()

from ..modules import module
if module.current_test:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This won't work for post install tests

@theangryangel
Copy link
Contributor

It looks interesting but right now I have difficulties to find a real use for odoo, since we have our own test environment (using log-db to get formatted logs as output).

Putting my developer and partner hat on, the testing suite output in Odoo is just frustrating to deal with, especially new hires and juniors who have experience with other testing frameworks which provide clearer output, rather than it all being mixed up in the logging output.

Having at least a table output at the end, or something would be really helpful. Independently I've been noodling on a similar set of functionality by parsing the output from the logs, but not yet had an opportunity to investigate.

Copy link
Contributor Author

@yajo yajo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your review @Xavier-Do

It looks interesting but right now I have difficulties to find a real use for odoo, since we have our own test environment (using log-db to get formatted logs as output).

The real use is that you could delete a lot of custom code! 😃

By using a standard XML report, you can just plug in one of the options outlined in first comment (or many others, compatible with this format), and it would produce a nice browsable unit test report.

It makes OdooTestResult inherit indirectly from TextTestResult, making it more dependant on unit test, we got some issue with that because of changes made in unittest breaking customisation we have. This is why we are trying to be more and more independent from unittest, and other libs.
[...]
As an example it looks like subTests are not well supported, and they are quite widely used in odoo. It would need some testing but I'm a bit skeptical right now.
This PR is just a PoC, not very polished.

I was trying to get an MVC to showcase how to:

  1. Get nicer output.
  2. Get XML reports.

I could attend the concerns from the review, but I won't do so if there are no chances to get a merge anyways, I hope you understand.

If we decide to add the tooling the output a test report in a parsable format, why should it be xUnit?

Probably a better implementation wouldn't depend on unittest-xml-reporting and would instead generate its own XML files, with support for subtests.

xUnit or Junit is simply the most standard, supported by all relevant tools.

I understand that it may be useful in some cases but I would prefer a less intrusive integration of this feature.
It may actually be interesting for developers if a local tool could display this result, didn't had the time to investigate.

The only alternative I can imagine is what @theangryangel said: some tool that parses odoo logs and produces a summary and the XML reports.

@@ -45,6 +45,7 @@ reportlab==3.6.8 ; python_version <= '3.10'
reportlab==3.6.12 ; python_version > '3.10'
requests==2.25.1 # versions < 2.25 aren't compatible w/ urllib3 1.26. Bullseye = 2.25.1. min version = 2.22.0 (Focal)
rjsmin==1.1.0
unittest-xml-reporting==3.2.0
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added it because I needed its base class to extend it and call super(), so it handles the creation of xunit reports.

@ap-wtioit
Copy link
Contributor

ap-wtioit commented Jan 31, 2024

@yajo

The only alternative I can imagine is what @theangryangel said: some tool that parses odoo logs and produces a summary and the XML reports.

There is https://github.com/camptocamp/pytest-odoo which can be used to run odoo tests with pytest, which we are using to generate the xUnit reports for Gitlab. And also trigger pytests directly from the IDE (PyCharm) by selecting a module / test file and click run tests.

Nonetheless i would prefer if Odoo was able to generate xUnit reports directly.

@yajo
Copy link
Contributor Author

yajo commented Jan 31, 2024

Modern Odoo can use --test-file to avoid having to reinstall an addon to test it, and --test-tags to target the test you want to run. Thus, the usefulness of pytest-odoo is very reduced. And quoting from its README:

Pytest-odoo can be considered a development tool, but not the tool that should replace entirely --test-enable in a CI.

It seemed to me like adding XML generation to Odoo was a simpler fix.

@pedrobaeza
Copy link
Collaborator

This can be very interesting to be added for Odoo partners that require extra CI systems for integration purposes (which are all the serious ones). runbot is a platform for testing only Odoo core itself, but it's not valid when you add in the equation OCA or custom modules.

@Xavier-Do please reconsider your position, although I agree this should be the less intrusive possible.

An alternative is being discussed here: https://github.com/orgs/OCA/discussions/167

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

6 participants