Skip to content

Conversation

@rsarm
Copy link
Contributor

@rsarm rsarm commented Oct 29, 2020

Fixes #1554

@rsarm rsarm added this to the ReFrame sprint 20.16 milestone Oct 29, 2020
@rsarm rsarm requested review from ekouts and vkarak October 29, 2020 16:01
@rsarm rsarm self-assigned this Oct 29, 2020
@codecov-io
Copy link

codecov-io commented Oct 29, 2020

Codecov Report

Merging #1556 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1556   +/-   ##
=======================================
  Coverage   88.03%   88.03%           
=======================================
  Files          44       44           
  Lines        6894     6897    +3     
=======================================
+ Hits         6069     6072    +3     
  Misses        825      825           
Impacted Files Coverage Δ
reframe/frontend/cli.py 78.76% <100.00%> (+0.17%) ⬆️

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 1fb7c64...6ee16e6. Read the comment docs.

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.

Now that I see the fix, I don't think that this was actually a bug, but rather the configuration was wrong. Nonetheless, this will prevent common user errors in the future. Apart from the rest of the comments, we will need to adjust the unit test configuration to exercise this. Simply change the following two lines:

https://github.com/eth-cscs/reframe/blob/master/unittests/resources/settings.py#L134-L135

to

                '-c unittests/resources/checks/hellocheck.py',
                '-p builtin-gcc',

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.

lgtm now

@teojgo teojgo added bugfix and removed bug labels Nov 3, 2020
@vkarak vkarak changed the title [bugfix] Fixing inconsistency in mode option passing [bugfix] Fix spacing issues when defining options for execution modes Nov 3, 2020
@vkarak vkarak merged commit 36e9635 into reframe-hpc:master Nov 3, 2020
@rsarm rsarm deleted the modes-cli-bug branch March 10, 2021 08:39
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.

Inconsistency in mode option passing

5 participants