Skip to content

Use constants for commonly used test easyconfig folders, files and content - #5203

Open
Flamefire wants to merge 1 commit into
easybuilders:developfrom
Flamefire:toy-and-test-dir-constants
Open

Use constants for commonly used test easyconfig folders, files and content#5203
Flamefire wants to merge 1 commit into
easybuilders:developfrom
Flamefire:toy-and-test-dir-constants

Conversation

@Flamefire

Copy link
Copy Markdown
Contributor

Followup to #5050

@boegel

boegel commented Aug 6, 2026

Copy link
Copy Markdown
Member

Yikes, this is pretty big...

@Flamefire Maybe we should more gradually adopt these constants, for example only update a single test module to use them, and then gradually work towards doing so in all test modules?

Otherwise this is going to be a nightmare to review...

@Flamefire

Copy link
Copy Markdown
Contributor Author

This is mostly a search & replace but in multiple steps. E.g.

-        test_ecs = os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs')
-        toy_ec = os.path.join(test_ecs, 't', 'toy', 'toy-0.0.eb')
-		use toy_ec
+		use TOY_EC
  1. Replace os.path.join(os.path.dirname(os.path.abspath(__file__)), 'easyconfigs', 'test_ecs') with TEST_ECS_DIR
  2. Replace os.path.join(test_ecs, 't', 'toy', 'toy-0.0.eb') with TOY_EC
  3. Replace toy_ec with TOY_EC
  4. remove useless assignments like TOY_EC = TOY_EC

Doing it gradually is not useful: Once the patterns are identified just search/replace over the test folder. If any tests fail revisit the pattern. This way we catch all instances and verify them.

The risk is low as it changes tests only (except for the 2 trivial changes to support Path and str arguments in 2 methods) and most changes themselves are trivial, e.g.:

-       test_ecs_dir = os.path.join(os.path.abspath(os.path.dirname(__file__)), 'easyconfigs', 'test_ecs')
-       ec_file = os.path.join(test_ecs_dir, 'f', 'foss', 'foss-2018a.eb')
+       ec_file = os.path.join(TEST_ECS_DIR, 'f', 'foss', 'foss-2018a.eb')
        ec = EasyConfig(ec_file)
        self.assertEqual(ec.dependency_names(), {'FFTW', 'GCC', 'OpenBLAS', 'OpenMPI', 'ScaLAPACK'})
        build_options = {
            'external_modules_metadata': ConfigObj(),
            'minimal_toolchains': True,
-           'robot_path': [test_ecs_dir],
+           'robot_path': [TEST_ECS_DIR],

I'd say it would be enough to just scroll over the changes and do only random spot checks.
Otherwise I can split this PR into multiple with 1 module each. However that would be >20 PRs and after the first one the changes in __init__.py will not be included in the diff so might make it harder if you want to go to the definition of e.g. TEST_MODULES_DIR
But of course can do if that helps getting them merged faster before it diverges again. Just tell me how many test modules you'd like per PR. Maybe 3?
Or maybe merge this now to avoid conflicts and divergence and you check each file once you have some time every now and then. In GitHub UI you can tick "Viewed" to keep track. Will be less work than checking 20 PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants