Skip to content

Conversation

@casparvl
Copy link

@casparvl casparvl commented May 4, 2021

If you use find_modules with an environment_mapping (as typically done in a flat module naming scheme) where one environment is a substring of the other (e.g. foss and fosscuda), find_modules will consider any regex specified for fosscuda to also be valid for foss. This PR fixes that.

Fixes #1960

@jenkins-cscs
Copy link
Collaborator

Can I test this patch?

@teojgo
Copy link
Contributor

teojgo commented May 4, 2021

ok to test

@codecov-commenter
Copy link

codecov-commenter commented May 4, 2021

Codecov Report

Merging #1961 (2ad0ace) into master (c118fec) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1961      +/-   ##
==========================================
- Coverage   87.91%   87.86%   -0.06%     
==========================================
  Files          50       50              
  Lines        8638     8668      +30     
==========================================
+ Hits         7594     7616      +22     
- Misses       1044     1052       +8     
Impacted Files Coverage Δ
reframe/utility/__init__.py 92.82% <0.00%> (ø)
reframe/frontend/cli.py 75.59% <0.00%> (-0.93%) ⬇️
reframe/frontend/statistics.py 95.45% <0.00%> (ø)
reframe/frontend/runreport.py 92.98% <0.00%> (+0.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c118fec...2ad0ace. Read the comment docs.

@vkarak vkarak added this to the ReFrame sprint 21.04.2 milestone May 4, 2021
@vkarak
Copy link
Contributor

vkarak commented May 4, 2021

ok to test

@vkarak vkarak requested review from victorusu and removed request for ekouts and teojgo May 5, 2021 21:16
Copy link
Contributor

@vkarak vkarak left a comment

Choose a reason for hiding this comment

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

Thanks @casparvl for your fix! It was indeed a bug. I think it has been a leftover from an earlier implementation that assumed lists. That's why you see environs in the for loop.

@vkarak vkarak changed the title [bugfix] Fixes potential issue with find_modules in flat module scheme [bugfix] Fixes find_modules() behaviour with flat module scheme when an environment is a substring of another one May 5, 2021
Copy link
Contributor

@victorusu victorusu left a comment

Choose a reason for hiding this comment

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

lgtm

@vkarak vkarak merged commit b457f82 into reframe-hpc:master May 6, 2021
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.

Unexpected DAG when using find_modules with environ-mappings in flat module scheme

6 participants