Skip to content

modules test not performing loads #436

Open

Description

@ben-bowers

Describe the bug

In an attempt to refactor modulefiles into separate common modulefiles, ModuleTests no longer operate as expected.
Presumably this is because when run in test-mode the dependent modulefiles are not loaded. Since the top level module relies on a setenv from the sub-modulefile, the variable evaluates incorrectly and the test fails.

This may be a feature rather than a bug - but if there is some workaround, that would be helpful to know.

To Reproduce

Steps to reproduce the behavior:

$ module test fake_tool
-------------------------------------------------------------------
Module Specific Test for /home/bbowers/modulefiles/fake_tool:

/fake/bin
Test result: FAIL

For modules v4.3 to v4.7, the output is:

_UNDEFINED_/fake/bin
Test result: FAIL

Location and content of any modulerc or modulefile involved:

$ cat fake_base_path 
#%Module1.0 # -*- tcl -*-
setenv BASE_PATH /home/bbowers

$ cat fake_tool
#%Module1.0 # -*- tcl -*-
module load fake_base_path
set toolpath [getenv BASE_PATH]/fake/bin
prepend-path PATH $toolpath
proc ModulesTest {} {
    global toolpath
    puts stderr $toolpath
    return [file exist $toolpath]
}

Expected behavior

It would be convenient if sub-modulefiles are loaded as part of the module test so that it is possible to refactor common parts of the modulefiles. If that is considered breaking behavior, then maybe a mode that enables loads/prereq during the test?

Error and debugging information

$ module --debug <command1> <arguments>
$ module --debug <command2> <arguments>
...

Modules version and configuration

Tested across several versions:

$ module --version
Modules Release 4.3.1 (2019-09-21)
Modules Release 4.7.1 (2021-04-06)
Modules Release 5.0.0 (2021-09-12)

Additional context

Activity

  1. adrien-cotte commented on Feb 16, 2022

    @adrien-cotte
    Contributor

    Hi,

    Could you explain why module load fake_base_path instead of prereq fake_base_path, please?

    Best,
    Adrien

  2. ben-bowers commented on Feb 16, 2022

    @ben-bowers
    Author

    @adrien-cotte It is my understanding that there is no real difference between module load and prereq if MODULES_AUTO_HANDLING=1.

    However, if that setting is not enabled then prereq requires that the sub-module load happened before loading the top level. Perhaps this is incorrect?

    In any event, I tested this case with both prereq and module load with modules 4.3.1 and MODULES_AUTO_HANDLING=1 and the result was the same.

  3. adrien-cotte commented on Feb 16, 2022

    @adrien-cotte
    Contributor

    Ok, sorry as my config always been auto_handling on, I forget this is not the default behavior :)

    Btw, I never test unloaded modules, is there any reason you cannot load your module before calling module test fake_tool?

    With your example:

    $ module test fake_tool
    -------------------------------------------------------------------
    Module Specific Test for /home/bbowers/modulefiles/fake_tool:
    
    /fake/bin
    Test result: FAIL
    -------------------------------------------------------------------
    
    $ module load fake_tool
    $ module test fake_tool
    -------------------------------------------------------------------
    Module Specific Test for /home/bbowers/modulefiles/fake_tool:
    
    /home/bbowers/fake/bin
    Test result: PASS
    -------------------------------------------------------------------
    

    I don't really know why we have to load modules before testing, maybe we should ask of Module mailing list?

    Does this workaround work for you?

  4. ben-bowers commented on Feb 16, 2022

    @ben-bowers
    Author

    In many (most?) cases it's probably not necessary to load the modulefile. Having to do so seems cumbersome to me, as you'd (perhaps) have to be careful to load, test and unload.

    Would the maintainers be interested in a PR to fix this issue? If so, is it necessary to be backward compatible with the existing behavior?

  5. xdelaruelle commented on Feb 17, 2022

    @xdelaruelle
    Collaborator

    The g_modfilePerModeAliases table in initModfileModeAliases defines how the modulefile commands are implemented for each evaluation mode:

    https://github.com/cea-hpc/modules/blob/3bc7b9904d7eb9448f9e2ff1c8e315afea53b8c1/tcl/mfinterp.tcl.in#L90-L130

    module, prereq, prereq-all, prereq-any, depends-on and conflict modulefile commands are currently implemented as a no-operation on test evaluation mode. test mode is made as close as help evaluation mode currently.

    PR would be interesting to add the proposed new behavior. I still need to think if this should be the new default behavior or if an option is needed to enable such change.

  6. adrien-cotte commented on Feb 17, 2022

    @adrien-cotte
    Contributor

    So, actually module test evaluation is close to module help?
    As I'm used to module load before module test, my opinion is that this PR makes sense.
    Maybe an option to select what should be not evaluated during tests. Default value as current behavior, until Module next big release?

  7. xdelaruelle commented on Mar 7, 2022

    @xdelaruelle
    Collaborator

    PR would be interesting to add the proposed new behavior. I still need to think if this should be the new default behavior or if an option is needed to enable such change.

    Maybe an option to select what should be not evaluated during tests. Default value as current behavior, until Module next big release?

    After some thought, I suggest the addition of a new configuration option, named test_mode_mimic (load or help as accepted values). Option should be set to help by default. When set to load, test evaluation mode should mimic a load evaluation mode, but not change user environment in the end of this processing.

    @ben-bowers You can follow the Add new configuration option guide to craft this pull request.

  8. ben-bowers commented on Jul 20, 2022

    @ben-bowers
    Author

    I believe I have something that barely works. The change to the Tcl is relatively small, but the files that needed to be updated were quite large. Thanks for the excellent Add New configuration option guide!

    One place I find that it is not working as expected is when I switch to use the load entry from the g_modfilePerModeAliases, the prereq command is working as in load mode, but the module load command is not.

    For instance, and the test_mode_mimic==load and the top modulefile is defined as:

    #%Module1.0 # -*- tcl -*-
    module load fake_base_path ## <--- doesnt evaluate this file
    set toolpath [getenv BASE_PATH]/fake/bin

    the fake_base_path is not evaluated.

    However when test_mode_mimic==load and the top modulefile is defined as:

    #%Module1.0 # -*- tcl -*-
    prereq fake_base_path ## <--- works!!
    set toolpath [getenv BASE_PATH]/fake/bin

    then the evaluation of the file happens.

    I had previously thought that these would have worked the same, but they call different tcl commands under the covers, so apparently it is not straightforward.

    The other side effects I need to figure out are:

    1. modules prints Loading fake_base_path, which is probably not something desired during test mode
    2. modules is actually loading the fake_base_path lower level file during test mode and setting the env-variables in the shell, which is not desirable
  9. xdelaruelle commented on Jul 21, 2022

    @xdelaruelle
    Collaborator

    For your first point, the module procedure (in main.tcl.in) adapts it behavior depending on the current evaluation mode found:

    https://github.com/cea-hpc/modules/blob/aa33eda3547ca070c83e6000bd9f0bd5e59bd229/tcl/main.tcl.in#L364-L365

    For the change your are crafting, I suggest you adapt the mode variable just below these code lines with:

    # set evaluation mode to the mode to mimic on test mode 
    if {$mode eq {test} && [getConf test_mode_mimic] eq {load}} {
       set mode load
    }

    This way module should operate in test mode as if was called in load mode if test_mode_mimic is set to load.

  10. xdelaruelle commented on Jul 21, 2022

    @xdelaruelle
    Collaborator

    For the second side effect mentioned, a clearSettings procedure could be added in envmngt.tcl.in file (above renderSettings procedure).

    This new procedure could array unset the following global arrays:

    • g_stateEnvVars
    • g_stateAliases
    • g_stateFunctions
    • g_stateCompletes
    • g_newXResources
    • g_delXResources

    Then the end of the module procedure should be updated (in main.tcl.in):
    https://github.com/cea-hpc/modules/blob/aa33eda3547ca070c83e6000bd9f0bd5e59bd229/tcl/main.tcl.in#L754-L756

    It should be updated to something like:

    if {$topcall} {
       # clear env changes if test mode mimics load mode
       if {[currentState mode] eq {test} && [getConf test_mode_mimic] eq\
          {load}} {
          clearSettings
       }                                                                                 
       renderSettings                                                                                     
    }
  11. xdelaruelle commented on Jul 21, 2022

    @xdelaruelle
    Collaborator

    For the Loading ... message I would need the PR code to see how the messages could be inhibited in this situation.

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

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions