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

Fix the test suite #1036

Merged
merged 14 commits into from
Apr 1, 2024
Merged

Conversation

picsel2
Copy link
Member

@picsel2 picsel2 commented Mar 1, 2024

This is a follow up on #999

The goal was to get the CI tests running again and later the scope was extended to fix the whole test suite.

@picsel2 picsel2 changed the title Fix tests Fix unit test CI job Mar 1, 2024
@picsel2 picsel2 force-pushed the fix_tests branch 3 times, most recently from e61007d to 5339e3c Compare March 1, 2024 01:20
@nekohayo nekohayo added the maintainability Automated tests suite, tooling, refactoring, or anything that makes it easier for developers label Mar 1, 2024
@picsel2 picsel2 force-pushed the fix_tests branch 11 times, most recently from 080a86b to 31fe22f Compare March 1, 2024 05:39
@picsel2 picsel2 changed the title Fix unit test CI job GHA: Get pytest to run tests again Mar 1, 2024
@picsel2 picsel2 marked this pull request as ready for review March 1, 2024 05:47
Copy link
Contributor

@SqAtx SqAtx left a comment

Choose a reason for hiding this comment

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

I can't run the tests locally right now, but if this diff takes us to a point where individual tests are failing instead of import errors etc, this is a step in the right direction :)

GTG/core/versioning.py Outdated Show resolved Hide resolved
@SqAtx
Copy link
Contributor

SqAtx commented Mar 2, 2024

It doesn't block the current changes, but I'm still getting import failures when running the tests locally:

$ ./run-tests 
==================================================================== test session starts =====================================================================
platform linux -- Python 3.11.6, pytest-8.0.1, pluggy-1.4.0
rootdir: /home/kevin/src/gtg
configfile: pyproject.toml
collected 151 items / 3 errors                                                                                                                               

=========================================================================== ERRORS ===========================================================================
___________________________________________________ ERROR collecting tests/backend/backend_caldav_test.py ____________________________________________________
ImportError while importing test module '/home/kevin/src/gtg/tests/backend/backend_caldav_test.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python3.11/importlib/__init__.py:126: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/backend/backend_caldav_test.py:11: in <module>
    from GTG.core.datastore import Datastore
GTG/core/datastore.py:37: in <module>
    import GTG.core.info as info
E   ModuleNotFoundError: No module named 'GTG.core.info'

(and 2 more)
It seems to not like the fact that the extension of https://github.com/getting-things-gnome/gtg/blob/master/GTG/core/info.py.in is .py.in maybe?

It might be a Python 3.11 thing - I haven't taken the time to install 3.8 locally. Nope, confirmed not a Python 3.11 thing - it fails the same way on Python 3.8.

Anyway, I can fix that separately.

@picsel2
Copy link
Member Author

picsel2 commented Mar 5, 2024

tldr

Install the project in a prefix of your choice using meson and add the directory with the installed package to PYTHONPATH. Run the tests with PYTHONSAFEPATH=1 PYTHONPATH="${prefix}/lib/python3.11/site-packages" ./run-tests

The lenghty explanation

Yeah... running locally is somewhat more complicated since GTG uses Meson to build its sources.

Configuring info.py

The file info.py.in is a template that gets filled (also called configured) by the meson setup step.
The resulting file is placed into the build directory.
This is the file we actually want Python to find and use.

Installing the project

Next we have to tell Python of the extra location to search for.
Unfortunately it is not enough to just add the build directory to PYTHONPATH.

The sources currently are a non-namespace package (PEP420).
This means here that we need all sources to be merged in one location since Python stops searching at the first GTG directory found, even if it does not contain a info.py file.

So instead we do a meson install and use the installed location in PYTHONPATH.

Disabling implicit search paths

You now might notice that Python still picks up the git sources, although PYTHONPATH points to the installed directory.
The reason is that Python silently adds the current working directory in front sys.path, rendering our previous efforts useless.
But you are in luck because Python 3.11 added a new flag that you can enable by setting PYTHONSAFEPATH=1.
It disables the implicit addition of the current directory to the search path. Exactly what we need!

@picsel2 picsel2 force-pushed the fix_tests branch 2 times, most recently from d628fa5 to a092ae2 Compare March 7, 2024 09:23
@picsel2 picsel2 requested a review from SqAtx March 7, 2024 10:51
Copy link
Contributor

@SqAtx SqAtx left a comment

Choose a reason for hiding this comment

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

2 minor comments but other than that, LGTM :)

Happy to have a green test run again! Thanks for your work there. I got it running locally as well. We'll add something to the README about that, because it's not obvious that we need to run Meson before we can run the tests.

GTG/core/datastore.py Outdated Show resolved Hide resolved
GTG/core/saved_searches.py Outdated Show resolved Hide resolved
GTG/core/saved_searches.py Outdated Show resolved Hide resolved
SqAtx and others added 14 commits March 16, 2024 14:51
Remove refactoring changes.
These only tested the design of the old weakly coupled attributes api.
It has been completely replaced an removed from the core.
Remove tree testing since Saved searches are not representing
hierarchies anymore.
This annotates the method as `staticmethod`, so it can be used and
tested again without requiring an instance of `ModifyTagsDialog`.
@diegogangl
Copy link
Contributor

Thank you for working on this! Great to see the checks green again :)

@diegogangl diegogangl merged commit 80b09c8 into getting-things-gnome:master Apr 1, 2024
1 check passed
@picsel2 picsel2 deleted the fix_tests branch April 9, 2024 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintainability Automated tests suite, tooling, refactoring, or anything that makes it easier for developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants