Skip to content

CMake symlink test data files #4076

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alexbeattie42
Copy link
Contributor

@alexbeattie42 alexbeattie42 commented May 7, 2025

The CMake test data file copy has never worked properly for me and I usually need to manually copy files around.

Brief summary of changes

Symlink the test data files to the target directory. This has the advantage of avoiding excessive copying and preserving changes made to the source files. As of CMake 3.13 (released in 2018) -E create_symlink is supported on all platforms and OpenSim uses version 3.15.

Testing I've completed

  • Ran locally
  • CI success indicates the files are linked properly

Looking for feedback on...

CHANGELOG.md (choose one)

  • no need to update because it is an internal change to configure CMake tests

This change is Reviewable

@alexbeattie42 alexbeattie42 force-pushed the alex/test-files-symlink branch from 60c1ea3 to 442d8da Compare May 7, 2025 18:54
@alexbeattie42 alexbeattie42 reopened this May 7, 2025
@alexbeattie42 alexbeattie42 force-pushed the alex/test-files-symlink branch from cc9ab19 to d045d18 Compare May 8, 2025 03:13
@alexbeattie42 alexbeattie42 force-pushed the alex/test-files-symlink branch from d045d18 to a5b697d Compare May 8, 2025 04:00
@alexbeattie42
Copy link
Contributor Author

@aymanhab @nickbianco could you take a look?

@aymanhab
Copy link
Member

aymanhab commented May 8, 2025

@alexbeattie42 If this actually works on windows that would be awesome and would solve a long standing issue with tests duplicating files. I Assume ci passing means it works on windows. If you have a local windows system to verify then please merge, otherwise let me know and I'll try to test it out before merging. Thank you 👍

@alexbeattie42
Copy link
Contributor Author

alexbeattie42 commented May 8, 2025

@alexbeattie42 If this actually works on windows that would be awesome and would solve a long standing issue with tests duplicating files. I Assume ci passing means it works on windows. If you have a local windows system to verify then please merge, otherwise let me know and I'll try to test it out before merging. Thank you 👍

Thanks! Unfortunately I don't have a Windows system setup for development at the moment as I mostly work in Linux. I have used this same technique in other projects that were cross platform (Windows, Mac x86, Mac ARM) and it worked well there. I did briefly peruse the logs to ensure that the tests actually did run and didn't somehow silently fail because of lack of files and it appears that they all ran properly. Here is the link to the docs where it's noted that it works in Windows as of 3.13 which I forgot to link above. I can try to get things up and running in Windows but I think it may take a bit more time than I'm expecting.

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.

2 participants