-
Notifications
You must be signed in to change notification settings - Fork 51
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
Use MATLAB caching in GitHub Actions #74
base: main
Are you sure you want to change the base?
Conversation
@kmruehl, @akeeste I've run a little experiment on using caching for the MATLAB install. You do get some savings, but my concern would be that if the caches are removed and are regenerated in a long-running test (for a PR for example) then an unexpected failure may occur due to a timeout. This timeout might never be fixable, as the cache won't be stored. Perhaps this needs another experiment to check what will happen in this edge case. EDIT: One way to deal with this would be to have a maximum time for any test of 30 minutes. There would then be time available to install MATLAB and build the caches before the 45-minute job timeout. We could enforce this by setting a timeout on the test running step, but this would generate a number of failures with the present test suite. |
Thanks @H0R5E revisiting the tests is something we plan to prioritize for v6.2. Thank you for looking into possible resolutions. |
@kmruehl, sorry that was not meant to happen. I track my PRs on a personal Trello board for my own project management. I had to reinstall the GitHub plugin, and I didn't realize it would add a comment the PRs by default. I've turned that feature off again now. Regarding this PR, the code is functional, but it's up to you whether you want to use it, given the pros and cons I have discussed. My advice would probably be not to use it because you can't guarantee that there won't be timeouts when doing cache building with the current test suite. If you can get all the tests below 30 minutes, then it would be safe to use, IMO. |
So, this one has been bugging me. It turns out that I set the job timeout to 45 minutes in this commit in order to avoid waiting for hung tests for the default of 6 hours. Sorry, I suffered a little bit of confirmation bias with this one - I had convinced myself this was a GitHub imposed limit. I think having a timeout to catch hung tests is sensible, but it was clearly in the wrong place. So, I've moved the timeout to the actual test running step. This will allow time for the caches to build and so this PR can be safely merged now. EDIT: Note that this doesn't change the fact that when the caches are built, test completion will be much slower. |
Also add a basic change to trigger a test
Find the envelope of MATLAB products for all tests and try to cache installation in order to reduce testing time.
Results
For the folder collection step:
For the Cable application example:
Discussion
Pros:
Cons:
If a long-running test is used to build the cache, it will likely time out<- because I defined it that way!Conclusion
When the caches are hit, a reduction in testing time of roughly 3-4 minutes is achieved. For some tests, this is a decent saving; however, this is a small percentage of the time required for the longer running tests in this repo.
Additionally, the significant increase to the testing time when the caches are being built might trigger unexpected failures if built during a long-running test. Whether this reduction in reliability of the test suite is worth it for the reduction in testing times is uncertain.