Skip to content

Conversation

@teojgo
Copy link
Contributor

@teojgo teojgo commented Jun 15, 2018

  • Allow setting the above RegressionTest fields during setup.

  • Create the corresponding unit test.

Closes #44

* Allow setting the above `RegressionTest` fields during setup.

* Create the corresponding unit test.
@codecov-io
Copy link

codecov-io commented Jun 15, 2018

Codecov Report

Merging #333 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #333      +/-   ##
=========================================
+ Coverage   91.29%   91.3%   +0.01%     
=========================================
  Files          67      67              
  Lines        8005    8018      +13     
=========================================
+ Hits         7308    7321      +13     
  Misses        697     697
Impacted Files Coverage Δ
unittests/test_pipeline.py 97.22% <100%> (+0.08%) ⬆️
reframe/core/pipeline.py 94.72% <100%> (+0.02%) ⬆️

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 8b48d8f...d8e119b. Read the comment docs.

if not self.current_system or not self._current_partition:
raise PipelineError('no system or system partition is set')

# Temporary fix for supporting multiple pre/post run steps
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefix this with FIXME: . Also remove the pre/post.

test.setup = custom_setup.__get__(test)

# Use test environment for the regression check
test.valid_prog_environs = [self.progenv.name]
Copy link
Contributor

Choose a reason for hiding this comment

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

You can simply use '*' here.

def custom_setup(obj, partition, environ, **job_opts):
obj.pre_run = ['echo prerun cmd']
obj.post_run = ['echo postrun cmd']
super(obj.__class__, obj).setup(partition, environ, **job_opts)
Copy link
Contributor

Choose a reason for hiding this comment

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

This test would have worked before your changes as well. You should try settings the pre_run post_run after calling the superclass setup. Also, you should echo slightly different strings from those defined in the setUp() method of the test. Otherwise, you cannot tell which ones were actually printed.

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. I will open an issue for creating an advanced tutorial example demonstrating this.

@vkarak vkarak changed the title Allow setting 'pre_run' and 'post_run' in 'setup' Enable setting 'pre_run' and 'post_run' in 'setup' Jun 18, 2018
@vkarak vkarak merged commit 2fc7c01 into master Jun 18, 2018
@vkarak vkarak deleted the enhancement/multiple_job_steps branch June 18, 2018 12:15
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.

4 participants