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

deprecate runtest in favor of testopts (was: Add pretestopts and testops) #2868

Open
wpoely86 opened this issue Sep 11, 2018 · 10 comments
Open
Milestone

Comments

@wpoely86
Copy link
Member

For #447 (EB 4.0)

  • We should add a pretestopts just like prebuildopts
  • Rename runtests to testopts

So we have %(pretestopts) make %(testopts) in the configuremake block

@boegel
Copy link
Member

boegel commented Sep 11, 2018

@wpoely86 Adding support for pretestopts and testopts should be done earlier, and then runtest should be deprecated.

If this is done early enough, we can drop support for runtest in EB 4.0 in favor of (pre)testopts.

The right place for this issue would have been in framework btw (although it does involve changing all uses of runtest to testopts in easyblocks/easyconfigs.

@boegel
Copy link
Member

boegel commented May 6, 2019

Support for (pre)testopts was added in #2793

@wpoely86 Do we still want to go ahead and deprecate runtest in favor of testopts for EasyBuild 4.0?

cc @akesandgren

@wpoely86
Copy link
Member Author

wpoely86 commented May 6, 2019

It would make sense?

@akesandgren
Copy link
Contributor

Sounds like a plan.

@boegel
Copy link
Member

boegel commented May 8, 2019

I guess it would, it's just a bit more work on top of other things for EasyBuild v4.0, which involves deprecating runtest in framework (in a way that doesn't break stuff that is still using it), and updating all easyblocks/easyconfigs that use it...

@boegel boegel transferred this issue from easybuilders/easybuild May 8, 2019
@boegel boegel added this to the 4.0 milestone May 8, 2019
@boegel
Copy link
Member

boegel commented May 22, 2019

I had a quick look at this, and deprecating runtest is less trivial than it may seem at first. The problem is that it means different things in different contexts...

    1. In the default EasyBlock.test_step implementation, it can be used to specify the full test command to run (for example runtest = "./run_tests.sh"). I'm not sure we should just use testopts in this case, since the name doesn't fit. Do we want to introduce a testcmd to replace this runtest interpretation?
    1. In ConfigureMake (and derivatives), you can specify the make target that should be used to run the tests (for example runtest = 'check' is equivalent with running make check); in this case we can just switch to testopts.
    1. runtest is expected to have a boolean value in various custom easyblocks, and is often set to True as default , indicating that the tests for that software to be run while leaving the door open to skip the tests using runtest = False. Also here testopts is not a good replacement. I guess here we can get away with recommending to use skipsteps = ['test'] as a replacement for runtest = False, or do we introduce a dedicated parameter for this interpretation too?

@akesandgren, @wpoely86: feedback welcome!

@boegel boegel changed the title Add pretestopts and testops deprecate runtest in favor of testopts (was: Add pretestopts and testops) May 22, 2019
@wpoely86
Copy link
Member Author

wpoely86 commented Jun 5, 2019

I agree on only using testopts for point ii.

@boegel
Copy link
Member

boegel commented Jun 9, 2019

@wpoely86 OK, but what do we do with use cases i) and iii) for which runtest is used now as well?

@wpoely86
Copy link
Member Author

I would keep it. It's a different thing and and not the thing I had in mind with testopts. This should go with buildopts etc. Point (i) is completely general and point (iii) is also independent (although a other name wouldn't have been a bad idea).

@boegel
Copy link
Member

boegel commented Sep 14, 2019

Getting rid of runtest won't be happening for EasyBuild v4.0.

We can limit the use cases of it over 4.x release, and perhaps make a more drastic change in EasyBuild v5.0...

@boegel boegel modified the milestones: 4.0.0, 4.x Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants