Skip to content

Fix potential failure in make test_install#434

Merged
shakedregev merged 1 commit into
developfrom
slaven/test-install-fix
Jun 5, 2026
Merged

Fix potential failure in make test_install#434
shakedregev merged 1 commit into
developfrom
slaven/test-install-fix

Conversation

@pelesh
Copy link
Copy Markdown
Collaborator

@pelesh pelesh commented Jun 4, 2026

Description

If make test_install is called before make install call, the test will fail giving misleading feedback that something is wrong with the installation configuration. See e.g. #428

Proposed changes

Enforce installation as a part of the target test_install. One-line fix suggested.

Checklist

  • All tests pass (make test and make test_install per testing instructions). Code tested on
    • CPU backend
    • CUDA backend
    • HIP backend
  • I have manually run the non-experimental examples and verified that residuals are close to machine precision. (In your build directory run: ./examples/<your_example>.exe -h to get instructions how to run examples). Code tested on:
    • CPU backend
    • CUDA backend
    • HIP backend
  • Code compiles cleanly with flags -Wall -Wpedantic -Wconversion -Wextra.
  • The new code follows Re::Solve style guidelines.
  • N/A There are unit tests for the new code.
  • N/A The new code is documented.
  • The feature branch is rebased with respect to the target branch.
  • N/A I have updated CHANGELOG.md to reflect the changes in this PR. If this is a minor PR that is part of a larger fix already included in the file, state so.

Further comments

If make install was not called, then
make test_install will fail. This fix
ensures make install is always
called before make_test install.
Copy link
Copy Markdown
Collaborator

@tamar-dewilde tamar-dewilde left a comment

Choose a reason for hiding this comment

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

I tested this with fresh CPU, CUDA, and HIP builds by running test_install without manually running install first. All three passed with local CMAKE_INSTALL_PREFIX values, and the fix looks correct to me.

Copy link
Copy Markdown
Collaborator

@andrewxu319 andrewxu319 left a comment

Choose a reason for hiding this comment

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

This works on a clean CPU/CUDA build. Called make test_install without make install.

@shakedregev
Copy link
Copy Markdown
Collaborator

Tested and it works. Nice clean fix.

@shakedregev shakedregev closed this Jun 5, 2026
@shakedregev shakedregev reopened this Jun 5, 2026
@shakedregev shakedregev merged commit d2897db into develop Jun 5, 2026
8 checks passed
@shakedregev shakedregev deleted the slaven/test-install-fix branch June 5, 2026 13:19
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.

4 participants