-
-
Notifications
You must be signed in to change notification settings - Fork 599
CI: Migrate test-new to meson #39641
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
Conversation
test-long: | ||
runs-on: ubuntu-latest | ||
needs: [test-new] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't remove this defeat the whole point of test-new? (that you do a fast-fail when there's a trivial error to notify the pull request author to fix it, so you don't burn CPU on testing the rest of the files)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main point of test-new
is that it provides quick feedback for devs. But I'm planning to also move these "long" tests to meson in a follow-up and then it's indeed a good idea to make run only after all other normal tests were passing.
By the way, the only benefit of ccache I can see is when you If you're already using docker then ccache should be useless. Also there's a risk of some hidden cache stale issue (there'd better be a rebuild from scratch every release at least) |
The meson workflow is not using docker and thus ccache is used to cache the compilation (more precisely: cython is still run on all cython files, but the compilation of the resulting c/c++ is cached). |
Documentation preview for this PR (built with commit 87caf60; changes) is ready! 🎉 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure why not, at least it's not slower than the current test-new.
The missing feature is "only run test-long (& full test with meson) if test-new succeeds" but… the only disadvantage is wasted electricity, I suppose.
Thanks @user202729 for the review. I'm setting this now to "positive review", hope you are okay with this. |
Wait a minute…
Any idea why it's like this? |
Shouldn't be caused by this PR; in fact, the checks for the commit for the last merge commit were okay. It is because the |
So… it's random? I guess if you retrigger CI it should be fine then? (why does the amount of disk usage fluctuate anyway? Sound like a bug somewhere…) |
sagemathgh-39641: CI: Migrate test-new to meson <!-- ^ Please provide a concise and informative title. --> <!-- ^ Don't put issue numbers in the title, do this in the PR description below. --> <!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method to calculate 1 + 2". --> <!-- v Describe your changes below in detail. --> <!-- v Why is this change required? What problem does it solve? --> <!-- v If this PR resolves an open issue, please link to it here. For example, "Fixes sagemath#12345". --> Migrates the `test-new` workflow to use Meson. The Meson build system rebuilds faster and leverages standard tools like `ccache` for caching Cython file compilation. The github workflow is also considerably easier to read and maintain now. To ensure reliability, pushing the Docker image in the build workflow has been disabled, as it was continuously clogging the GitHub cache (limited to 10GB, while each cached image consumes several gigabytes). This change also resolves intermittent workflow failures caused by unsuccessful image pushes. ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [ ] The title is concise and informative. - [ ] The description explains in detail what this PR is about. - [ ] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39641 Reported by: Tobias Diez Reviewer(s): Tobias Diez, user202729
sagemathgh-39867: Run long test in test-new sagemath#39641 introduced an issue that long new tests are not ran in test-new. This is an issue because then the author is only notified by the failure when test-long finishes. (e.g. happened to me in sagemath#39733 .) ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39867 Reported by: user202729 Reviewer(s): Tobias Diez
sagemathgh-39867: Run long test in test-new sagemath#39641 introduced an issue that long new tests are not ran in test-new. This is an issue because then the author is only notified by the failure when test-long finishes. (e.g. happened to me in sagemath#39733 .) ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39867 Reported by: user202729 Reviewer(s): Tobias Diez
sagemathgh-39867: Run long test in test-new sagemath#39641 introduced an issue that long new tests are not ran in test-new. This is an issue because then the author is only notified by the failure when test-long finishes. (e.g. happened to me in sagemath#39733 .) ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39867 Reported by: user202729 Reviewer(s): Tobias Diez
sagemathgh-39867: Run long test in test-new sagemath#39641 introduced an issue that long new tests are not ran in test-new. This is an issue because then the author is only notified by the failure when test-long finishes. (e.g. happened to me in sagemath#39733 .) ### 📝 Checklist <!-- Put an `x` in all the boxes that apply. --> - [x] The title is concise and informative. - [x] The description explains in detail what this PR is about. - [x] I have linked a relevant issue or discussion. - [ ] I have created tests covering the changes. - [ ] I have updated the documentation and checked the documentation preview. ### ⌛ Dependencies <!-- List all open PRs that this PR logically depends on. For example, --> <!-- - sagemath#12345: short description why this is a dependency --> <!-- - sagemath#34567: ... --> URL: sagemath#39867 Reported by: user202729 Reviewer(s): Tobias Diez
Migrates the
test-new
workflow to use Meson. The Meson build system rebuilds faster and leverages standard tools likeccache
for caching Cython file compilation. The github workflow is also considerably easier to read and maintain now.To ensure reliability, pushing the Docker image in the build workflow has been disabled, as it was continuously clogging the GitHub cache (limited to 10GB, while each cached image consumes several gigabytes). This change also resolves intermittent workflow failures caused by unsuccessful image pushes.
📝 Checklist
⌛ Dependencies