Skip to content

Add pytest command to run tests in CI workflow #40474

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

Merged
merged 7 commits into from
Aug 2, 2025

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Jul 24, 2025

Alternative to #40446. This time running pytest directly in the CI, instead of as part of sage -t.

Removed one old test file that fails with meson since the sage cli is different then.

📝 Checklist

  • 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

@tobiasdiez tobiasdiez mentioned this pull request Jul 24, 2025
5 tasks
@user202729
Copy link
Contributor

sage -t is still supported, no? I use it all the time, even with meson editable install.

What's the issue with the deleted test?

@tobiasdiez
Copy link
Contributor Author

sage -t is still supported, no? I use it all the time, even with meson editable install.

What's the issue with the deleted test?

The sage script in the root does provide -t, but not the sage cli that is installed by meson. The tests in CI are picking up only the installed sage script.

The tests are more or less outdated/not very important.

@user202729
Copy link
Contributor

I'd rather be careful and do a # needs !meson. (Is the feature flag already there?)

@tobiasdiez
Copy link
Contributor Author

With #39030 there is only going to be a meson build...

@orlitzky
Copy link
Contributor

Is src/conftest_inputtest.py still useful after this? And the exception for it in conftest.py?

@tobiasdiez
Copy link
Contributor Author

Is src/conftest_inputtest.py still useful after this? And the exception for it in conftest.py?

Good observation. It's no longer used, indeed, and thus I've removed it now as well.

@orlitzky
Copy link
Contributor

Tobias is the one who added these tests, so "they're not very important" is a more compelling argument than usual. In the near future we will (want to) be able to tell people to run pytest rather than a weird sage command, so removing the test for the old way (that breaks under the new way) seems reasonable to me.

@orlitzky
Copy link
Contributor

Not related, but is it intended that the Meson/Windows jobs are passing even though the test phase fails?

@orlitzky
Copy link
Contributor

FWIW there's no way to test for meson per se, but there are several variables that are meaningless outside of sage-the-distro and are usually unset (distro packages tend to have similar problems).

@tobiasdiez
Copy link
Contributor Author

Thanks for the review!

Not related, but is it intended that the Meson/Windows jobs are passing even though the test phase fails?

Yes, as we don't have any meaningful way to run the tests on Windows now (in a way that make most of them pass). The important part is that the compilation is successful on Win.

FWIW there's no way to test for meson per se, but there are several variables that are meaningless outside of sage-the-distro and are usually unset (distro packages tend to have similar problems).

With #39030, sage-the-distro will also finally use the meson backend. Afterwards we can indeed remove all those special variables, and also determine features during build time and not runtime.

@vbraun vbraun merged commit 783445b into sagemath:develop Aug 2, 2025
20 of 23 checks passed
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.

4 participants