Skip to content

AMReX Smoke Test#27411

Merged
tldahlgren merged 16 commits intospack:developfrom
etpalmer63:amrex_smoke_test
Jan 13, 2022
Merged

AMReX Smoke Test#27411
tldahlgren merged 16 commits intospack:developfrom
etpalmer63:amrex_smoke_test

Conversation

@etpalmer63
Copy link
Copy Markdown
Contributor

This PR puts in place a stand-alone test for AMReX. It is only compatible with the development branch at this point, however it will be supported on future releases of AMReX. I also had to implement the CMake workaround to ensure the same version that was used to install AMReX is used to compile the stand-alone test.

I have several concerns regarding skipping tests for past AMReX versions and would appreciate feedback on how to address them. They are:

  1. On line 276: I'm just using print to explain that the test has been skipped. Should I be sending a warning or doing something else instead?
  2. In general, our test is not compatible with older versions and thus is skipped. However, when I run spack test run amrex the status is a reported as "PASSED". Is there a way to report a different status, such as "SKIPPED", etc. ?

Thanks!

Erik

@spackbot-app spackbot-app bot added new-version stand-alone-tests Stand-alone (or smoke) tests for installed packages update-package labels Nov 12, 2021
@tldahlgren
Copy link
Copy Markdown
Contributor

  1. On line 276: I'm just using print to explain that the test has been skipped. Should I be sending a warning or doing something else instead?

At this point we've been asking people to simply print that the test is skipped. We've discussed pytest-like skip functionality but have not had the cycles to complete yet.

  1. In general, our test is not compatible with older versions and thus is skipped. However, when I run spack test run amrex the status is a reported as "PASSED". Is there a way to report a different status, such as "SKIPPED", etc. ?

This is a known issue and the current approach is to print a skipped message. See answer to 1 for more.

WeiqunZhang
WeiqunZhang previously approved these changes Nov 16, 2021
Copy link
Copy Markdown
Member

@WeiqunZhang WeiqunZhang left a comment

Choose a reason for hiding this comment

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

LGTM. I tested it locally. It works as expected.

@etpalmer63
Copy link
Copy Markdown
Contributor Author

I made the changes as requested. Thanks everyone for your input!

Copy link
Copy Markdown
Member

@ax3l ax3l left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @etpalmer63! 🎉

@ax3l ax3l requested a review from tldahlgren January 4, 2022 12:19
@tldahlgren tldahlgren enabled auto-merge (squash) January 12, 2022 17:18
@tldahlgren
Copy link
Copy Markdown
Contributor

Force re-running checks due to package-audits stalled reporting.

@tldahlgren tldahlgren closed this Jan 13, 2022
auto-merge was automatically disabled January 13, 2022 17:32

Pull request was closed

@tldahlgren tldahlgren reopened this Jan 13, 2022
@tldahlgren tldahlgren enabled auto-merge (squash) January 13, 2022 17:32
@tldahlgren tldahlgren merged commit eda565f into spack:develop Jan 13, 2022
RikkiButler20 pushed a commit to RikkiButler20/spack that referenced this pull request Feb 1, 2022
EthanS94 pushed a commit to EthanS94/spack that referenced this pull request Feb 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-version stand-alone-tests Stand-alone (or smoke) tests for installed packages update-package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants