Skip to content

Conversation

@vkarak
Copy link
Contributor

@vkarak vkarak commented Jun 17, 2022

This PR improves how tests handle the container_platform attribute. Users are no more required to set the container_platform in their tests. This will be taken from the default container_platform defined for the current partition in the configuration. Also, this PR brings back the workdir attribute of the container platform and each backend emits the right option.

The documentation and the tutorial are also adapted and a new tutorial example is added which show cases a tests of GROMACS that runs both natively and from different containers.

Implementation details

In order to allow setting the container_platform attributes in the class body, we have employed the following trick. We assign a bogus ContainerPlatform that implements its abstract methods simply by raising a NotImplementedError. The purpose of this container platform is to hold any attributes set by the user. During the test's setup, when we know the current system partition, we replace the container_platform by creating a concrete one and copying any set attributes from the bogus. This whole mechanism is completely transparent to the users, which they can also set explicitly the container_platform if they wish, as in the past.

Fixes #2396.

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

@codecov-commenter
Copy link

Codecov Report

Merging #2537 (3035adc) into master (fbb95a5) will increase coverage by 0.08%.
The diff coverage is 87.71%.

❗ Current head 3035adc differs from pull request most recent head dbf45db. Consider uploading reports for the commit dbf45db to get more accurate results

@@            Coverage Diff             @@
##           master    #2537      +/-   ##
==========================================
+ Coverage   85.81%   85.89%   +0.08%     
==========================================
  Files          58       58              
  Lines       10798    10867      +69     
==========================================
+ Hits         9266     9334      +68     
- Misses       1532     1533       +1     
Impacted Files Coverage Δ
reframe/core/pipeline.py 93.05% <73.58%> (-0.44%) ⬇️
reframe/core/containers.py 100.00% <100.00%> (+1.96%) ⬆️
reframe/core/systems.py 90.72% <100.00%> (+1.23%) ⬆️
reframe/utility/__init__.py 92.60% <100.00%> (+0.10%) ⬆️

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 fbb95a5...dbf45db. Read the comment docs.

@vkarak vkarak merged commit c4a270b into reframe-hpc:master Jun 22, 2022
@vkarak vkarak deleted the feat/cont-platf-agnostic-tests branch June 22, 2022 13:41
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.

Make tests container-runtime agnostic

4 participants