OCA / pylint-odoo

Odoo plugin for Pylint
http://www.pylint.org
143 stars 168 forks source link

[REF] Improved repo structure to facilitate testing #445

Closed antonag32 closed 1 year ago

antonag32 commented 1 year ago

The existence of src was not necessary, and made it difficult to run unittests directly, since the imports depended on pylint_odoo being installed in the system, instead of importing directly from source code, so tox was the only way to run tests (AFAIK).

After this change, unittests can be directly run without tox, this makes the development process faster, and integrates very well with IDEs that let you run and debug unit tests directly, plus it removes something that is not needed.

pylint itself uses this structure, so I think that is another reason in favor of this change.

antonag32 commented 1 year ago

This is just a side PR, as I am working on detecting unused python files (not imported but should have), however I wanted to run unittests directly and was not able to which lead me to this. I am aware the REF might be too much for now, but might as well put it out here before I forget about it.

moylop260 commented 1 year ago

@antonag32

FYI we are using the following convention related to src folder:

Notice tests is brother of src but I preferred remove them in order to avoid adding to the pypi package heavy test_repo folder then tests one

It is an old and long discussion related to this folder but we have decided to have it TL: DR;

Also, recently the unittest was passing because it was assuming you are able to import packages that the package released is not

So, it is better an isolated environment (tox) with minimal requirements in order to detect packaging errors early

FYI I have added to README the commands to run the unittest:

I have detected packaging errors related to this:

It was fixed with the following code:

Similar case for "ImportError: No module named 'packaging'":

It was a false green because the package packaging was installed in the OS environment but using an isolated environment it is detected early

requirements.txt file missing

The last 2 ones was fixed with the following fix:

So, I even think the src path is good and running the unittest in isolated environment is even better

moylop260 commented 1 year ago

Also, notice the update-readme tox env requires a particular version of python in order to avoid mutable results based on different versions

--help output is slightly different for each version of python the isolated environment needs to pin the python version too

antonag32 commented 1 year ago

Fair enough. Thanks for the context and explanation. I wanted to use the integration Pycharm has by default which lets you run single tests (by just clicking on the green arrow):

Screenshot from 2022-11-18 15-43-05

However they use unittest without Tox and the import shown in the original comment fails, no big deal, I will continue using the pytest -k flag with Tox.