-
-
Notifications
You must be signed in to change notification settings - Fork 123
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
Running tests on post_install #28
Comments
cc @OCA/core-maintainers |
A little bit reading https://en.wikipedia.org/wiki/Unit_testing |
These are tests, not unit tests. You give that meaning with your approach. |
We can't do integration tests in OCA repositories at module level ! We have been always done unit tests. Or am I wrong ? |
We are not talking about doing integration tests at OCA level, but on integrator level, but OCA modules mustn't forbid this possibility. And the same tests are used for integration tests. |
I don't have the full discussion context, but as general rule OCA repos should test all the modules installed together. |
I am wondering whether this repository is the right one for the discussion: do you think we should have a separate document with the guidelines or would you like to have the discussion first here and then push a guideline? |
@elicoidal I suggested to raise the discussion here because a guideline is needed and the guideline will end up in CONTRIBUTING.rst in this repo. |
I also think, we are more closer in OCA to unit test than integration tests. However, the function we are testing have a lot of inputs (a lot of records to prepare) thus it is simpler to do the workflow instead of mocking every inputs. That's why I advocate to skip the code in test context with check on test-enable to not force other modules' tests to run it. But maybe this could be an opt-in option in res.config.settings making the default workflow the first to be in use in other modules tests. Which would force you to install a module + enable an option. @dreispt I'm not sure if you advocate for more resilient tests or to just ensure that modules don't break others with fairness. Of course fairness in module design doesn't help on testing module compatibility. For more testing especially integration tests to check compatibility of modules with all activated features, I would suggest to create glue modules containing tests that enable all features and proves two or more independent modules are working well together. In some case it could be very useful. And there you would explicitly be testing the unrelated modules. IMO when creating a module, a rule of thumb should be that standard workflow can always be followed by other modules' tests by default. And if your module doesn't comply it deserves test isolation. |
Ensuring that modules don't break other, as a general rule, is a basic requirement, and the main reason why we should have functional-area focused repos. Integration tests involving several different modules would be an ideal scenario, but I agree with you that it's not easy and add that there's not much motivation for that when the modules involved have different authors. Yes, "glue" test modules for integration tests would be a way for this, so no technical impediment for actually doing it.
Absolutely. That perfectly sums up my point. |
I would like to add an additional argument to respect the normal flow in the execution of tests. It happens regularly in my developments to run the tests of a single module after having modified it. In some cases, I discover that even without my modifications, the tests fail because the code tries to access fields or methods that are not present in dependencies. These errors are systematically masked by our CI since the addons providing these stuff are installed by others addons. If the tests are executed in post_install, this kind of bugs will also be masked.... |
But this is something that you are not going to fully avoid removing post_install: it will depend on the module loading order. |
In the best world, each module should be tested in its own env with only these dependencies installed. Even if with our large repositories it's not possible to fully avoid this but with post_install we are sure that this kind of error will no more be detected. |
Would it be a sensible guideline to say that OCA module that impose restrictions or changes on standard flows must have a flag (off by default) to enable that behaviour? In any case, I don't see a good reason to have all OCA tests having the post_install flag by default. With my superficial view on the issue, it seems to me that it only postpones the problem. |
FYI in my MQT earlier work I included a "unit test" build where each module was individually tested. |
The problem was not the need of it, but the time it consumes. |
As I triggered this, I want to know if we can summarize the result of this thread. As repos are grouping most modules allowing to extend functionnalities in a same way, but as OCA development involves many authors that expect their modules working not necessarily with all of those contained in the repo, letting the default behaviour (at_install) should be the more 'convenient' way not to exhaust contributors to fix neverending errors. And to ensure adequation between two fighting ones should be developping glue modules, options to enable changing behaviour, or ... That way could lead to more independance (as for now) between modules. Is this could be inserted into development guideline ? |
I just opened #63 and then later I found this thread, so you might be interested in reviewing that PR, which clarifies the situation in the specific case of website modules for Odoo v12+. |
As debated here : OCA/server-ux#32 and OCA/delivery-carrier#167 (comment) I want to have clear guidelines on how to run tests on OCA repositories. At post install, at install, or ... because it can leads to unproductive threads between different modules maintainers.
Many thanks for your feedback, experience, ...
The text was updated successfully, but these errors were encountered: