Skip to content

make ConfigureMake, MesonNinja and SCons generic easyblock aware of pretestopts#1645

Merged
boegel merged 2 commits intoeasybuilders:developfrom
akesandgren:use_pretestopts
Mar 12, 2019
Merged

make ConfigureMake, MesonNinja and SCons generic easyblock aware of pretestopts#1645
boegel merged 2 commits intoeasybuilders:developfrom
akesandgren:use_pretestopts

Conversation

@akesandgren
Copy link
Copy Markdown
Contributor

No description provided.

@akesandgren akesandgren requested a review from boegel March 3, 2019 11:28
@akesandgren akesandgren added this to the 3.x milestone Mar 3, 2019
@akesandgren
Copy link
Copy Markdown
Contributor Author

This makes easybuilders/easybuild-easyconfigs#7762 work if one adds

pretestopts = 'export OMP_NUM_THREADS=1 && '

Copy link
Copy Markdown
Member

@boegel boegel left a comment

Choose a reason for hiding this comment

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

Same remark applies to other tweaked easyblocks as well.


if self.cfg['runtest']:
cmd = "make %s" % (self.cfg['runtest'])
cmd = "%s make %s" % (self.cfg['pretestopts'], self.cfg['runtest'])
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@akesandgren Also pick up on testopts while you're at it?

cmd = ' '.join([self.cfg['pretestopts'], "make %s" % self.cfg['runtest'], self.cfg['testopts']])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I left that out because you can basically set runtest to include any othe options you need to have.
So I'm not sure that there is a reason for testopts.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In some contexts, runtest can be a boolean (I think for Python packages), so we should allow both runtest and testopts to compose the test command, I see no reason not to.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok, will fix for the ones i changed already. Leaving the more complicated ones for later.

@boegel boegel modified the milestones: 3.x, next release (3.8.2) Mar 3, 2019
@akesandgren akesandgren requested a review from vanzod March 12, 2019 11:35
@boegel boegel removed the request for review from vanzod March 12, 2019 12:31
@boegel
Copy link
Copy Markdown
Member

boegel commented Mar 12, 2019

lgtm, thanks @akesandgren!

@boegel boegel merged commit 0ba9a87 into easybuilders:develop Mar 12, 2019
@akesandgren akesandgren deleted the use_pretestopts branch March 12, 2019 12:43
@boegel boegel changed the title Use the newly added pretestopts in test_step where applicable. make ConfigureMake, MesonNinja and SCons generic easyblock aware of pretestopts Apr 11, 2019
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.

2 participants