Skip to content

Conversation

@rsarm
Copy link
Contributor

@rsarm rsarm commented Jan 21, 2021

Closes #1700

@rsarm rsarm added this to the ReFrame sprint 21.01 milestone Jan 21, 2021
@rsarm rsarm requested review from teojgo and vkarak January 21, 2021 16:08
@rsarm rsarm self-assigned this Jan 21, 2021
@teojgo teojgo changed the title [bugfix] Job options passed from the command-line are appended to self.job.options defined in the test [bugfix] Append cli job options to the ones defined in the test Jan 21, 2021
Copy link
Contributor

@teojgo teojgo left a comment

Choose a reason for hiding this comment

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

lgtm

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 except that we should probably use a better name than sched_options inside the Job.

@codecov-io
Copy link

Codecov Report

Merging #1701 (d8d41ca) into master (74f009f) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1701   +/-   ##
=======================================
  Coverage   87.31%   87.31%           
=======================================
  Files          46       46           
  Lines        7639     7640    +1     
=======================================
+ Hits         6670     6671    +1     
  Misses        969      969           
Impacted Files Coverage Δ
reframe/core/launchers/mpi.py 98.76% <100.00%> (ø)
reframe/core/schedulers/__init__.py 97.79% <100.00%> (+0.01%) ⬆️
reframe/core/schedulers/pbs.py 47.58% <100.00%> (ø)
reframe/core/schedulers/slurm.py 53.78% <100.00%> (ø)

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 74f009f...d8d41ca. 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.

lgtm

@vkarak vkarak changed the title [bugfix] Append cli job options to the ones defined in the test [bugfix] Ensure that job options passed from the command-line are always the last in the generated job script Jan 21, 2021
@vkarak vkarak merged commit 294ec08 into reframe-hpc:master Jan 21, 2021
@rsarm rsarm deleted the bugfix/job-options branch March 10, 2021 08:38
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.

self.job.options override any job options passed from the command-line

4 participants