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

Fix for running tests with use= flags enabled #4363

Merged
merged 1 commit into from
Jul 11, 2023

Conversation

dipinhora
Copy link
Contributor

Prior to this commit, running make test after running make configure use=use_flag would fail because the Makefile would look for the test binaries in the wrong directory (i.e. in build/debug instead of build/debug-runtimestats).

This commit updates the Makefile to try and determine the use output directory from the CMake configure generated cmake_install.cmake file so that tests can be run successfully. It will fall back to the current behavior and use the default build directory if it the file doesn't exist or it is unable to extract the directory path.

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Jul 6, 2023
@jemc
Copy link
Member

jemc commented Jul 7, 2023

I don't feel qualified to comment on CMake improvements, so I'm requesting review from @chalcolith.

@jemc jemc requested a review from chalcolith July 7, 2023 18:41
@SeanTAllen
Copy link
Member

@dipinhora can you rebase this against main?

Prior to this commit, running `make test` after running
`make configure use=use_flag` would fail because the `Makefile`
would look for the test binaries in the wrong directory (i.e. in
`build/debug` instead of `build/debug-runtimestats`).

This commit updates the `Makefile` to try and determine the `use`
output directory from the CMake configure generated
`cmake_install.cmake` file so that tests can be run successfully.
It will fall back to the current behavior and use the default
build directory if it the file doesn't exist or it is unable to
extract the directory path.
@dipinhora
Copy link
Contributor Author

@dipinhora can you rebase this against main?

done

@SeanTAllen SeanTAllen merged commit b6c3c61 into ponylang:main Jul 11, 2023
18 checks passed
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants