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 support for Environment Modules as modules tool to pass unit tests with v4.2+ #4369

Merged
merged 4 commits into from
Dec 27, 2023

Conversation

xdelaruelle
Copy link
Contributor

@xdelaruelle xdelaruelle commented Oct 29, 2023

On my way to work on #4368 I have found that some unit tests (test_avail, test_load and test_get_setenv_value_from_modulefile) are failing with Environment Modules v4.2+.

Here are some changes to either update:

  • a unit test (test_load) that was expecting a different behavior for Modules 4.2+
  • EnvironmentModules class to better cope with newer Modules versions

test_load expects that version 4.2+ of Environment Modules reload an
already loaded module. As of version 5.3, already loaded modules are not
reloaded unless one of their dependent module is unloaded or loaded.
Defines environment variables when initializing EnvironmentModules
object to ensure that this module tool will not be influenced by
external configuration.

Setup Environment Modules configuration to ensure module search behaves
like EasyBuild expects (match module name start, case sensitive, return
in-depth modulepath content, ignore cache file). Also defines a basic
output configuration not to get unexpected content like tags or
variants.

This change helps to pass "test_avail" test with Environment Modules
v5.0+.
Starting Environment Modules 4.2, value of setenv statement in "module
show" output is enclosed in curly braces if it contains spaces.

This change helps to pass "test_get_setenv_value_from_modulefile" test
with Environment Modules v4.2+
@boegel boegel changed the title Fix EnvironmentModules to pass unit tests fix support for Environment Modules as modules tool to pass unit tests with v4.2+ Nov 8, 2023
@boegel boegel added this to the 4.x milestone Nov 8, 2023
Copy link
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

@xdelaruelle Let's also update .github/workflows/unit_tests.yml to test with a more recent version of Environment Modules, to verify that these changes are sufficient to pass the tests (and to help ensure that we don't accidentally break support for recent Environment Modules later)?
Just one or two extra "key" versions is enough, maybe we should also stop testing with the now ancient v4.1.4 (we shouldn't have too many test configurations).

@xdelaruelle
Copy link
Contributor Author

@xdelaruelle Let's also update .github/workflows/unit_tests.yml to test with a more recent version of Environment Modules, to verify that these changes are sufficient to pass the tests (and to help ensure that we don't accidentally break support for recent Environment Modules later)? Just one or two extra "key" versions is enough, maybe we should also stop testing with the now ancient v4.1.4 (we shouldn't have too many test configurations).

@boegel This is what I have prepared in another PR that I will push as soon as #4369 and #4371 will get merged. These two PRs are needed for the CI to pass with Environment Modules v4.2+.

EL8 is now shipping Environment Modules v4.5, so I will propose to change the v4.1.4 key by v4.5

@boegel boegel modified the milestones: 4.x, next release (4.9.0) Dec 26, 2023
@boegel boegel merged commit d977648 into easybuilders:develop Dec 27, 2023
39 checks passed
@xdelaruelle xdelaruelle deleted the modules_unit_tests branch December 27, 2023 12:05
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