Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

first attempt at a common staging class #209

Closed
wants to merge 3 commits into from

Conversation

smoors
Copy link
Collaborator

@smoors smoors commented Nov 24, 2024

update: also added a test-agnostic sanity check to the staging class



@rfm.simple_test
class EESSI_CP2K(rfm.RunOnlyRegressionTest, EESSI_Mixin):

stage_files = fixture(EESSI_CP2K_stage_input, scope='session')
srcdir = os.path.join(os.getcwd(), 'cp2k_staging', 'src')
stage_files = fixture(EESSI_Staging, scope='session', variables={'sourcesdir': srcdir})
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if this will work. If another test uses EESSI_Staging as a fixture, with scope session, will it then be run twice (i.e. once for each tests invoking it as a fixture)? Or once (because it's once per session scope)?

Also: even if it does run once per test that invokes this as fixture, it'll have the name EESSI_Staging both times, right? That could be confusing. But maybe that can be resolved by overwriting the test name through the variables construct? I don't know, I don't have much experience with variables as an argument to fixture.

Could you try to add the lammps case here as well? That would allow us to easily verify it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Discussed in the meeting: maybe we can do something along the lines of:

stage_files = fixture(EESSI_Staging, scope='session', variables={'sourcesdir': srcdir, 'testname': testname + '_staging'})

@casparvl
Copy link
Collaborator

Discussed, we'll use readonly_files #211 and simply accept that we cannot run from a nodelocal dir (requires adaptations according to #210 )

@casparvl casparvl closed this Nov 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants