pumi: fix path to smoketest input data #32446

cwsmith · 2022-08-30T17:24:39Z

This PR fixes the path to the input data needed to successfully run spack test run pumi.

spackbot-app · 2022-08-30T17:28:37Z

It looks like you had an issue with style checks! I can help with that if you ask me! Just say:

@spackbot fix style

... and I'll try to fix style and push a commit to your fork with the fix.

Alternatively, you can run:

$ spack style --fix

And then update the pull request here.

spackbot-app · 2022-08-30T17:28:41Z

It looks like you had an issue with style checks! I can help with that if you ask me! Just say:

@spackbot fix style

... and I'll try to fix style and push a commit to your fork with the fix.

Alternatively, you can run:

$ spack style --fix

And then update the pull request here.

tldahlgren

Looks good though testing in the install prefix creates a problem for some use cases.

tldahlgren · 2022-08-31T14:12:24Z

var/spack/repos/builtin/packages/pumi/package.py

        exe = "uniform"
-        options = ["../testdata/pipe.dmg", "../testdata/pipe.smb", "pipe_unif.smb"]
+        options = ["../share/testdata/pipe.dmg", "../share/testdata/pipe.smb", "pipe_unif.smb"]
        expected = "mesh pipe_unif.smb written"
        description = "testing pumi uniform mesh refinement"
        self.run_test(exe, options, expected, purpose=description, work_dir=self.prefix.bin)


Stand-alone tests should not be run from the prefix due to issues with permissions in read-only installations. This can be avoided by either providing the full path to the files in the prefix or by copying the test data to the test stage and using the full path to executable(s). For example,

Suggested change

exe = "uniform"

options = ["../testdata/pipe.dmg", "../testdata/pipe.smb", "pipe_unif.smb"]

options = ["../share/testdata/pipe.dmg", "../share/testdata/pipe.smb", "pipe_unif.smb"]

expected = "mesh pipe_unif.smb written"

description = "testing pumi uniform mesh refinement"

self.run_test(exe, options, expected, purpose=description, work_dir=self.prefix.bin)

data_dir = self.prefix.share.testdata

exe = self.prefix.bin.uniform

options = [

join_path(data_dir, "pipe.dmg"),

join_path(data_dir, "pipe.smb"),

join_path(self.prefix.bin, "pipe_unif.smb"),

]

expected = "pipe_unif.smb written"

description = "testing pumi uniform mesh refinement"

self.run_test(exe, options, expected, purpose=description)

tldahlgren · 2022-08-31T14:22:10Z

var/spack/repos/builtin/packages/pumi/package.py

@@ -109,15 +109,15 @@ def test(self):
        if self.spec.version <= Version("2.2.6"):
            return
        exe = "uniform"
-        options = ["../testdata/pipe.dmg", "../testdata/pipe.smb", "pipe_unif.smb"]
+        options = ["../share/testdata/pipe.dmg", "../share/testdata/pipe.smb", "pipe_unif.smb"]
        expected = "mesh pipe_unif.smb written"
        description = "testing pumi uniform mesh refinement"
        self.run_test(exe, options, expected, purpose=description, work_dir=self.prefix.bin)

        mpiexec = Executable(join_path(self.spec["mpi"].prefix.bin, "mpiexec")).command


You don't need to pass an executable to run_test so you could simply set this to:

Suggested change

mpiexec = Executable(join_path(self.spec["mpi"].prefix.bin, "mpiexec")).command

mpiexec = self.spec["mpi"].prefix.bin.mpiexec

tldahlgren · 2022-08-31T14:24:14Z

var/spack/repos/builtin/packages/pumi/package.py

        expected = "mesh pipe_unif.smb written"
        description = "testing pumi uniform mesh refinement"
        self.run_test(exe, options, expected, purpose=description, work_dir=self.prefix.bin)

        mpiexec = Executable(join_path(self.spec["mpi"].prefix.bin, "mpiexec")).command
        mpiopt = ["-n", "2"]
        exe = ["split"]
-        options = ["../testdata/pipe.dmg", "../testdata/pipe.smb", "pipe_2_.smb", "2"]
+        options = ["../share/testdata/pipe.dmg", "../share/testdata/pipe.smb", "pipe_2_.smb", "2"]
        expected = "mesh pipe_2_.smb written"
        description = "testing pumi mesh partitioning"
        self.run_test(


As stated above, the test should not be executed in the installation directory so you want to remove the work_dir argument.

tldahlgren · 2022-08-31T14:26:02Z

var/spack/repos/builtin/packages/pumi/package.py

        expected = "mesh pipe_unif.smb written"
        description = "testing pumi uniform mesh refinement"
        self.run_test(exe, options, expected, purpose=description, work_dir=self.prefix.bin)

        mpiexec = Executable(join_path(self.spec["mpi"].prefix.bin, "mpiexec")).command
        mpiopt = ["-n", "2"]
        exe = ["split"]
-        options = ["../testdata/pipe.dmg", "../testdata/pipe.smb", "pipe_2_.smb", "2"]
+        options = ["../share/testdata/pipe.dmg", "../share/testdata/pipe.smb", "pipe_2_.smb", "2"]


You can provide full paths here:

Suggested change

options = ["../share/testdata/pipe.dmg", "../share/testdata/pipe.smb", "pipe_2_.smb", "2"]

options = [

join_path(data_dir, "pipe.dmg"),

join_path(data_dir, "pipe.smb"),

"pipe_2_.smb",

"2",

]

tldahlgren

See prior suggestions and comments.

cwsmith · 2022-11-23T19:22:40Z

Hi @tldahlgren,

I made the suggested edits (outside github) and was able to run spack test run pumi@2.2.7 without any obvious issues.

wspear · 2023-01-18T14:56:23Z

var/spack/repos/builtin/packages/pumi/package.py

-        exe = ["split"]
-        options = ["../testdata/pipe.dmg", "../testdata/pipe.smb", "pipe_2_.smb", "2"]
-        expected = "mesh pipe_2_.smb written"
+        mpiexec = self.spec["mpi"].prefix.bin.mpiexec


Is there a case where the full path to the mpi executable is necessary? There are software environments where only the mpi compilers are provided in the mpi bin directory and the executors are located elsewhere. This test is breaking for us in that situation. Also, mpiexec is not always available. I recommend using something like this:

mpiexe_list = ["mpirun", "mpiexec", "srun"] for mpiexe in mpiexe_list: if which(mpiexe) is not None: mpiexec = Executable(mpiexe).command break

I'm not sure. If the full path was not set, how would the spack test mechanism locate the mpi executor that matches the mpi install used for the build? I haven't tried mixing an mpi executor from one mpi install with libraries/headers from another.

The correct mpi environment would have to be loaded for the test to run successfully so it would pick up the executor from the path. I think that is generally how spack smoke tests are implemented. This isn't to support mixing and matching mpi components but a single mpi with an atypical directory structure.

Sounds good. Thank you. I'll make the change.

Quick spack/python question. Should we handle the case where mpirun, mpiexec or srun is not found? If so, what is the clean way to report the error and exit?

That would be safer. Maybe skip the mpi test with a warning rather than a failure in that case? Ideally we should have an mpi executor method provided by spack so every mpi enabled test doesn't have to reimplement this...

We have tests that pick the first of those executables it finds and runs with it. If you want to demonstrate running with all available, you could do that.

(FYI. I'm refactoring stand-alone testing support so maybe I can sneak this in. 😉 )

alecbcs · 2023-11-19T00:25:57Z

@cwsmith did you get a chance to take a look at the review feedback. Is this PR still something you'd like to pursue?

cwsmith · 2023-11-21T19:55:24Z

@alecbcs Yes. Sorry. I'll get back to this after the holiday.

@tldahlgren

thank you @tldahlgren

spackbot-app bot added the update-package label Aug 30, 2022

cwsmith mentioned this pull request Aug 30, 2022

Spack test fails: "gmi failed: could not open model file" SCOREC/core#370

Closed

cwsmith marked this pull request as ready for review August 31, 2022 01:23

tldahlgren reviewed Aug 31, 2022

View reviewed changes

tldahlgren self-assigned this Aug 31, 2022

tldahlgren requested changes Aug 31, 2022

View reviewed changes

cwsmith requested a review from tldahlgren November 23, 2022 19:21

wspear requested changes Jan 18, 2023

View reviewed changes

wspear mentioned this pull request Jan 23, 2023

[support]: pumi test failure on JLSE E4S-Project/e4s#109

Closed

wspear mentioned this pull request Feb 2, 2023

[support]: slate - spack test failure E4S-Project/e4s#114

Closed

cwsmith and others added 5 commits January 10, 2024 10:34

pumi: fix path to smoketest input data

8d35a84

double quotes

071a34d

pumi: test dir fixes

f4f7309

thank you @tldahlgren

pumi: fix mpi test paths

9682347

pumi: fix style

19c0208

cwsmith force-pushed the cws/pumiFixSmokeTest branch from 1289a40 to 19c0208 Compare January 10, 2024 15:35

Angelyr and others added 2 commits January 11, 2024 14:31

find mpi exec (#42050)

63ba7db

fix style

9dead10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pumi: fix path to smoketest input data #32446

pumi: fix path to smoketest input data #32446

cwsmith commented Aug 30, 2022

spackbot-app bot commented Aug 30, 2022

spackbot-app bot commented Aug 30, 2022

tldahlgren left a comment

tldahlgren Aug 31, 2022

tldahlgren Aug 31, 2022

tldahlgren Aug 31, 2022

tldahlgren Aug 31, 2022

tldahlgren left a comment

cwsmith commented Nov 23, 2022

wspear Jan 18, 2023

cwsmith Jan 18, 2023 •

edited

wspear Jan 18, 2023

cwsmith Jan 18, 2023

cwsmith Jan 18, 2023 •

edited

wspear Jan 18, 2023

tldahlgren Jan 18, 2023

alecbcs commented Nov 19, 2023

cwsmith commented Nov 21, 2023

	mpiexec = Executable(join_path(self.spec["mpi"].prefix.bin, "mpiexec")).command
	mpiexec = self.spec["mpi"].prefix.bin.mpiexec

pumi: fix path to smoketest input data #32446

Are you sure you want to change the base?

pumi: fix path to smoketest input data #32446

Conversation

cwsmith commented Aug 30, 2022

spackbot-app bot commented Aug 30, 2022

spackbot-app bot commented Aug 30, 2022

tldahlgren left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tldahlgren left a comment

Choose a reason for hiding this comment

cwsmith commented Nov 23, 2022

Choose a reason for hiding this comment

cwsmith Jan 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cwsmith Jan 18, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alecbcs commented Nov 19, 2023

cwsmith commented Nov 21, 2023

cwsmith Jan 18, 2023 •

edited

cwsmith Jan 18, 2023 •

edited