enable runtest and test_cmd customization in MesonNinja easyblock - #4111
enable runtest and test_cmd customization in MesonNinja easyblock#4111Thyre wants to merge 6 commits into
runtest and test_cmd customization in MesonNinja easyblock#4111Conversation
runtest and test_cmd customization in MesonNinja easyblock
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
| test_cmd = self.cfg.get('test_cmd') or DEFAULT_TEST_CMD | ||
| runtest = self.cfg['runtest'] |
There was a problem hiding this comment.
@Thyre We need to be a bit more careful here I think, to avoid introducing a backwards-incompatible change...
For an exiting easyconfig file like cairomm-1.16.2-GCC-12.3.0.eb which uses MesonNinja as easyblock and includes this currently:
runtest = 'ninja test'this (and the logic below) means trouble, because we're essentially prepending "meson " to it, which results in:
== testing...
>> running shell command:
export MESON_TESTTHREADS=16 && export MESON_NUM_PROCESSES=16 && meson ninja test
Our easyconfig files are being updated so they work nicely with this updated MesonNinja easyblock in easybuilders/easybuild-easyconfigs#25749, but we need to take into account that we don't control all easyconfig files out there.
We should try and come up with an approach where existing easyconfigs that include runtest = 'ninja test' keep working as expected...
There was a problem hiding this comment.
test_cmd should only be set to the default when runtest is True.
For all other values, we should keep this empty.
This should keep the old behavior for existing external EasyConfigs.
Co-authored-by: Kenneth Hoste <kenneth.hoste@ugent.be>
Signed-off-by: Jan André Reuter <j.reuter@fz-juelich.de>
|
Test report by @Thyre Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (total: 1 min 38 secs) (1 easyconfigs in total) |
|
Test report by @Thyre Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (total: 46 secs) (1 easyconfigs in total) |
|
HPCToolkit (using One drawback of the current implementation is that But first, I want to do some more testing. |
Signed-off-by: Jan André Reuter <j.reuter@fz-juelich.de>
|
@boegelbot please test @ jsc-zen3 |
|
@Thyre: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 4759051325 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 33 out of 36 (total: 2 hours 20 mins 42 secs) (36 easyconfigs in total) |
|
I'd guess that the test failure is unrelated? The last two are still due to MATLAB missing on jsc-zen3. |
|
@boegelbot please test @ jsc-zen3 |
|
@Thyre: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... Details- notification for comment with ID 4760011115 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot Overview of tested easyconfigs (in order)
Build succeeded for 1 out of 1 (total: 6 mins 31 secs) (1 easyconfigs in total) |
| else: | ||
| test_cmd = '' |
There was a problem hiding this comment.
I think we might want to give a warning if we are overriding a test_cmd set in the EC file.
Maybe for another PR but in the configuremake EB we use both test_cmd and runtest, would have to check other EBs but we might want to make this behavior more consistent (maybe even move the general test logic at the framework level)
(created using
eb --new-pr)Instead of relying on each EasyConfig to set
runtest = ninja test, provide a sane default and enable users of the EasyBlock to just setruntest = Truefor a sane default. Also enable customization viatest_cmd, available in other generic easyblocks.To further ensure that we're not using too many resources, limit the parallelism via environment variables described in the Meson documentation. Just calling
meson testwould use all available CPU cores, ignoring our wanted parallelism.Closes #4087