Rework python version handling associated testconfig path handling. #134

albu-diku · 2024-10-14T22:10:46Z

A previous revision fixed an immediate problem with test path handling
but knowingly left some things on the table - in particular a split
between container based handling of Python 2 and the local execution
of Python 3 (meaning a potentially inconsistent version relative to
what the project considers officially supported).

Address this entirely: rework the default behaviour of make test to
container based execution and use a consistent baseline Python 3.

In order to continue to support rapid local iteration, provide a
separate make unittest target which will execute the test suite
locally and is also used as a mechanism to run other supporting tools.

The clean use of containers necessitated various changes that make path
arguments within the testconfig non-overlapping and better isolated.
Adjust all paths generated within the test suite to be always from the
base root instead of relative the output directory. Opt to patch the
makeconfig generator rather than fiddle with generateconfs again.

While here also add support for an environment variable override that
allows execution of the test suite against arbitrary python 3 versions.

A previous revision fixed an immediate problem with test path handling but knowingly left some things on the table - in particular a split between container based handling of Python 2 and the local execution of Python 3 (meaning a potentially inconsistent version relative to what the project considers officially supported). Address this entirely: rework the default behaviour of `make test` to container based execution and use a consistent baseline Python 3. In order to continue to support rapid local iteration, provide a separate `make unittest` target which will execute the test suite locally and is also used as a mechanism to run other supporting tools. The clean use of containers necessitated various changes that make path arguments within the testconfig non-overlapping and better isolated. Adjust all paths generated within the test suite to be always from the base root instead of relative the output directory. Opt to patch the makeconfig generator rather than fiddle with generateconfs again. While here also add support for an environment variable overried that allows execution of the test suite against arbitrary python 3 versions.

jonasbardino · 2025-01-16T15:57:53Z

envhelp/docker/Dockerfile.py3

@@ -0,0 +1,8 @@
+FROM python:3.9


I wonder if this comes from an implicit assumption that we prefer python-3.9 due to Rocky9 defaults. Perhaps we should make a brief comment about it if so. Just to make it easier to track once we reach Rocky10.
The alternative would be to pull in a full rocky9 container instead but that may be overkill.
It probably makes little practical difference but if possible we could maybe even track the precise 3.9.N version:

[root@next ~]# cat /etc/redhat-release Rocky Linux release 9.5 (Blue Onyx) [root@next ~]# python -V Python 3.9.21

It tends to remain fixed until the next major distro release.

jonasbardino · 2025-01-16T15:59:41Z

envhelp/makeconfig.py


    overrides = {
        'destination': os.path.join(_LOCAL_ENVHELP_OUTPUT_DIR, confs_name),
        'destination_suffix': "-%s" % (confs_suffix,),
    }

-    # determine the paths by which we will access the various configured dirs
-    if is_py2:
+    # determine the paths b which we will access the various configured dirs


Minimal typo, which I can easily fix during merge.

jonasbardino · 2025-01-16T16:07:56Z

envhelp/python3

@@ -2,7 +2,7 @@
 #
 # --- BEGIN_HEADER ---
 #
-# python3 - wrap python3 virtual environment for testing
+# python2 - wrap python2 docker container for testing


You didn't really mean 2 here, right? Looks copy/pasted from python2 wrapper, so I'll fix during merge unless you protest.

jonasbardino · 2025-01-16T16:21:00Z

tests/support/__init__.py


-PY2 = (sys.version_info[0] == 2)
+from tests.support._env import MIG_ENV, PY2


Nitpick: I think we should avoid (especially) leading underscores in module names.
If we don't want it exposed directly in tests.support I think we should e.g. put it in a sub-dir and have support __init__ expose only relevant variables explicitly.
Just a piece of general advice and input for the discussed style guide :-)

jonasbardino · 2025-01-16T16:26:01Z

tests/support/__init__.py

@@ -146,7 +152,11 @@ def tearDown(self):
            if os.path.islink(path):
                os.remove(path)
            elif os.path.isdir(path):
-                shutil.rmtree(path)
+                try:
+                    shutil.rmtree(path)


Should we insert an assertion here to check that no stray dangerous paths are in play (think '.' or similar inadvertently ending up in _cleanup_paths)?

... where '.' is just an example, which could also be the absolute path of the migrid clone / root directory when one takes the _register_path assertion about abspath below into account.

jonasbardino · 2025-01-16T16:40:56Z

tests/support/__init__.py

-        tmp_path = os.path.join(TEST_OUTPUT_DIR, relative_path)
-    else:
+
+    if os.path.isabs(relative_path):


A bit tongue-in-cheek nitpicking, but perhaps we should rename relative_path in a follow-up as it clearly may be absolute in actual use sometimes.

.github/workflows/ci.yml

jonasbardino

Thanks, it looks good and appears to work as intended in practice. Preparing to merge.

jonasbardino · 2025-01-17T10:20:29Z

I rebased on edge to pull in github hook updates and polished a bit as spelled out in the attached patch as merge preparation. Just to address the minor purely non-code points above.
PR134-polish.txt

…are only part of the specific github branches. Reworks python version handling associated testconfig path handling. Credits for the original unit test framework and this unification work to move all tests into containers goes out to Alex Burke (albu-diku). Thanks, Alex! :) I only did a bit of minor polish as explained in #134 git-svn-id: svn+ssh://svn.code.sf.net/p/migrid/code/trunk@6195 b75ad72c-e7d7-11dd-a971-7dbc132099af

jonasbardino · 2025-01-17T15:16:55Z

Thanks, Alex 👍
Merged through svn except the github CI actions merged directly in edge and experimental branches where those bits live.

jonasbardino · 2025-01-17T15:18:47Z

Marked with follow-up pending. Please to go over the comments above and see if anything should be addressed in separate PRs.

albu-diku mentioned this pull request Oct 15, 2024

Minimal fix to ensure isolation of filesystem accesses by the test suite. #135

Closed

albu-diku force-pushed the refactor/test-config-paths-redux branch from 5e7692c to f531944 Compare October 29, 2024 16:46

albu-diku changed the title ~~Refactor/test config paths redux~~ Rework est configuration paths Oct 30, 2024

albu-diku force-pushed the refactor/test-config-paths-redux branch from 8277ed5 to edc8c96 Compare November 2, 2024 10:22

albu-diku changed the title ~~Rework est configuration paths~~ Rework python version handling associated testconfig path handling. Nov 2, 2024

albu-diku marked this pull request as ready for review November 2, 2024 10:30

This was referenced Nov 2, 2024

Use the correct force_utf8 function based on Python version. #140

Open

ALBU tracking #96

Closed

albu-diku mentioned this pull request Nov 12, 2024

Make createuser work for new installations and add basic coverage. #120

Closed

albu-diku force-pushed the refactor/test-config-paths-redux branch from edc8c96 to 5e59ee7 Compare November 18, 2024 10:58

This was referenced Jan 11, 2025

Implement support code for the testing of WSGI handlers/servers. #175

Open

Basic coverage of migwsgi. #98

Open

jonasbardino added enhancement New feature or request unit test Code and infrastructure related to unit testing labels Jan 16, 2025

jonasbardino added this to the Python3 support for general research data sites milestone Jan 16, 2025

jonasbardino self-assigned this Jan 16, 2025

jonasbardino reviewed Jan 16, 2025

View reviewed changes

.github/workflows/ci.yml Show resolved Hide resolved

jonasbardino approved these changes Jan 16, 2025

View reviewed changes

jonasbardino closed this Jan 17, 2025

jonasbardino added the follow-up pending Pending tasks to follow-up on after close label Jan 17, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rework python version handling associated testconfig path handling. #134

Rework python version handling associated testconfig path handling. #134

albu-diku commented Oct 14, 2024 •

edited

Loading

jonasbardino Jan 16, 2025

jonasbardino Jan 16, 2025

jonasbardino Jan 17, 2025

jonasbardino Jan 16, 2025

jonasbardino Jan 17, 2025

jonasbardino Jan 16, 2025

jonasbardino Jan 16, 2025

jonasbardino Jan 16, 2025

jonasbardino Jan 16, 2025

jonasbardino left a comment

jonasbardino commented Jan 17, 2025 •

edited

Loading

jonasbardino commented Jan 17, 2025

jonasbardino commented Jan 17, 2025


		PY2 = (sys.version_info[0] == 2)
		from tests.support._env import MIG_ENV, PY2

Rework python version handling associated testconfig path handling. #134

Rework python version handling associated testconfig path handling. #134

Conversation

albu-diku commented Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonasbardino left a comment

Choose a reason for hiding this comment

jonasbardino commented Jan 17, 2025 • edited Loading

jonasbardino commented Jan 17, 2025

jonasbardino commented Jan 17, 2025

albu-diku commented Oct 14, 2024 •

edited

Loading

jonasbardino commented Jan 17, 2025 •

edited

Loading