Test suite improvements

[Flamefire]

Extracted some parts of #3780

Major change: Running the suite now allows to pass any valid unittest args, such as -k for filtering or --verbose for more detailed output (really missed that in #3780) similar to https://github.com/easybuilders/easybuild-easyconfigs/commit/17379dbe0150caccf4b5a949b8d2ef6b9d0354bf in the EC repo

• [ ] Requires #4188
• [x] Requires #4204

[Flamefire]

I remember something about https://github.com/easybuilders/easybuild-framework/pull/3790/commits/9d04087034ef6f9932d7375c5f5800b111237517 The "proper" way just marks the test as skipped and hence is no longer caught by the filtered output check. I.e. previously if a test was skipped it would print something to stdout which the later check would find and fail CI. This no longer works (this way).

So maybe the check should look for skipped tests? Or just ignore this for now...

[boegel]

These changes break the way we're currently filtering tests from test.framework.suite, which we should try and avoid:

$ python3 -m test.framework.suite load
E
======================================================================
ERROR: load (unittest.loader._FailedTest)
----------------------------------------------------------------------
AttributeError: module '__main__' has no attribute 'load'

----------------------------------------------------------------------
Ran 1 test in 0.000s

FAILED (errors=1)

This is supposed to filter all tests down to just the ones that include load in the test name...

[Flamefire]

These changes break the way we're currently filtering tests from test.framework.suite, which we should try and avoid:

But they allow the proper way: python3 -m test.framework.suite -k load should work which it did not before especially for the non-standard way of adding subtests. See e.g. https://github.com/easybuilders/easybuild-framework/pull/3790/commits/4e0a269d0402609d7424c76506c5db6aba638c0f

Edit: BTW: The failing test is due to a faulty usage env_file = open(reprod_dumpenv, "r").read(). Replace this by read_file to avoid the resource leak (missing close for open)

[boegel]

These changes break the way we're currently filtering tests from test.framework.suite, which we should try and avoid:

But they allow the proper way: python3 -m test.framework.suite -k load should work which it did not before especially for the non-standard way of adding subtests. See e.g. 4e0a269

OK, but can we somehow automagically translate python3 -m test.framework.suite load to python3 -m test.framework.suite -k load, to ensure both work?

Many people are used to filtering tests by just passing a pattern, and this still works for the individual subsuites:

python3 -m test.framework.modules load

(so does -k load here)

[Flamefire]

OK, but can we somehow automagically translate python3 -m test.framework.suite load to python3 -m test.framework.suite -k load, to ensure both work?

Not sure. I'm afraid that would be hacky and could be ambiguous and/or break things. As far as I can tell this way now is the "correct" way (i.e. as documented by unittest) so if people now have to pass an additional -k I guess they'll manage ;) How many are running the EB test suite anyway? And if we have to decide between having access to (all) the regular unittest flags or this little backwards compat thing I'd say the change is well worth it

[Flamefire]

Rebased and conflicts resolved

[Flamefire]

@boegel Any news here?

[Flamefire]

Rebased after #4066 got merged

[Flamefire]

CI Test currently fails with Python 3.10 due to use of LooseVersion:

FAIL: test_end2end_singularity_image (test.framework.containers.ContainersTest)
  File "/tmp/runner/5c8d89e5eb3ef5612a2540f36e5e8cfb5e8d5efb/lib/python3.10/site-packages/test/framework/containers.py", line 301, in test_end2end_singularity_image
    self.assertFalse(stderr)
AssertionError: "/tmp/runner/5c8d89e5eb3ef5612a2540f36e5e8cfb5e8d5efb/lib/python3.10/site-packages/easybuild/tools/containers/utils.py:85: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.\n  version_ok = LooseVersion(str(min_tool_version)) <= LooseVersion(tool_version)\n/tmp/runner/5c8d89e5eb3ef5612a2540f36e5e8cfb5e8d5efb/lib/python3.10/site-packages/easybuild/tools/containers/singularity.py:364: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.\n  if LooseVersion(singularity_version) > LooseVersion('3.0'):" is not false

See #3794 for a fix

[Flamefire]

@boegel I split this into smaller PRs which should be easier to check so the individual improvements can be merged although I'd like to get this one in here despite the change that now -k is required (but additional unittest arguments are now supported)

[boegel]

@Flamefire All split of PRs are merged now, I think, can you sync this with current develop?

[boegel]

@Flamefire This is still quite big, is there more we can reasonably flesh out in somewhat smaller PRs perhaps, to make this easier to digest? I guess we should get #4170 merged first though, that seems like "low hanging fruit", although it's also a large PR (but probably easier to review)

[Flamefire]

Yes I was able to extract 4 PRs. See the mentions above