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

allow use of test_cmd without runtest for ConfigureMake #2837

Merged

Conversation

Flamefire
Copy link
Contributor

@Flamefire Flamefire commented Nov 29, 2022

(created using eb --new-pr)

When using CMakeNinja and running tests one needs test_cmd = 'ctest' and runtest = '' which is akward but required because it simply concats runtest even when it is not set (None)

Also handle the default case for test_cmd in the test function so derived classes can detect if the command was set by the user or not. Required for #2838

And finally clean up the executed command by omitting empty values (e.g. pre/posttestopts not set which resulted in " ctest ")

@Flamefire Flamefire mentioned this pull request Nov 29, 2022
@boegel boegel changed the title Allow use of test_cmd without runtest for ConfigureMake allow use of test_cmd without runtest for ConfigureMake Dec 7, 2022
@boegel boegel added this to the 4.x milestone Dec 7, 2022
boegel
boegel previously requested changes Dec 7, 2022
easybuild/easyblocks/generic/configuremake.py Outdated Show resolved Hide resolved
easybuild/easyblocks/generic/configuremake.py Outdated Show resolved Hide resolved
@Flamefire Flamefire requested a review from boegel June 29, 2023 13:37
@Flamefire
Copy link
Contributor Author

Anything missing here?

@boegel
Copy link
Member

boegel commented Sep 13, 2023

Anything missing here?

@Flamefire Similar to #2838: a battery of tests with existing easyconfigs that either use ConfigureMake directly, or an easyblock that derives from it, mostly.

@Flamefire
Copy link
Contributor Author

I started test runs for both with some random ECs using ConfigureMake/CMakeMake respectively, hope it doesn't fail for unrelated reasons as I don't expect any issues by these PRs as explained in #2838 (comment)

@Flamefire
Copy link
Contributor Author

Test report by @Flamefire

Overview of tested easyconfigs (in order)

  • SUCCESS arpack-ng-3.8.0-foss-2022b.eb
  • SUCCESS QwtPolar-1.1.1-GCCcore-10.2.0.eb
  • SUCCESS ICU-69.1-GCCcore-11.2.0.eb
  • SUCCESS ICU-72.1-GCCcore-12.2.0.eb
  • SUCCESS Vim-9.0.0950-GCCcore-11.3.0.eb
  • SUCCESS cURL-7.78.0-GCCcore-11.2.0.eb
  • SUCCESS cURL-7.83.0-GCCcore-11.3.0.eb
  • SUCCESS cURL-7.86.0-GCCcore-12.2.0.eb
  • SUCCESS cairo-1.16.0-GCCcore-11.2.0.eb
  • SUCCESS c-ares-1.18.1-GCCcore-11.2.0.eb
  • SUCCESS CoinUtils-2.11.6-GCC-11.2.0.eb
  • SUCCESS libdap-3.20.8-GCCcore-11.2.0.eb
  • SUCCESS libxml2-python-2.9.13-GCCcore-11.3.0.eb
  • SUCCESS xproto-7.0.31-GCCcore-11.2.0.eb
  • SUCCESS ITSTool-2.0.7-GCCcore-11.3.0.eb
  • SUCCESS imake-1.0.8-GCCcore-11.2.0.eb

Build succeeded for 16 out of 16 (14 easyconfigs in total)
taurusi8034 - Linux CentOS Linux 7.9.2009, x86_64, AMD EPYC 7352 24-Core Processor, 8 x NVIDIA NVIDIA A100-SXM4-40GB, 470.57.02, Python 2.7.5
See https://gist.github.com/Flamefire/9c92d1fabdd65cac48c620fe6bef8771 for a full test report.

Copy link
Contributor

@akesandgren akesandgren left a comment

Choose a reason for hiding this comment

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

LGTM

@akesandgren
Copy link
Contributor

akesandgren commented Sep 14, 2023

Test report by @akesandgren

Overview of tested easyconfigs (in order)

  • SUCCESS Amber-22.0-foss-2021b-AmberTools-22.3.eb

Build succeeded for 1 out of 1 (1 easyconfigs in total)
b-an02.hpc2n.umu.se - Linux Ubuntu 20.04, x86_64, Intel(R) Xeon(R) CPU E5-2690 v4 @ 2.60GHz, Python 3.8.10
See https://gist.github.com/akesandgren/bcccb9a6f0910ea04a240647e915a94a for a full test report.

This was with both 2837 and 2838

@akesandgren
Copy link
Contributor

Going in, thanks @Flamefire!

@akesandgren akesandgren merged commit 9f29e4e into easybuilders:develop Sep 14, 2023
@Flamefire Flamefire deleted the 20221129132443_new_pr_configuremake branch September 14, 2023 11:35
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.

3 participants