Skip to content

Conversation

antonio-rojas
Copy link
Contributor

Instead of sage -t, which no longer works with meson

Copy link

github-actions bot commented Jun 28, 2025

Documentation preview for this PR (built with commit 7ff88f2; changes) is ready! 🎉
This preview will update shortly after each push to this PR.

"""
cmd = "sage -t"
cmd = "sage-runtests" if "sage-runtests" in argv[0] else "python3 -m sage.doctest"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change it from sage -t to sage-runtests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to minimize the wrapping layers - I didn't make it python3 -m sage.doctest unconditionally because sage may not be installed in the system python namespace

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay!

@tobiasdiez
Copy link
Contributor

CI is failing now, seems like a typo in the merge resolution.

@antonio-rojas
Copy link
Contributor Author

antonio-rojas commented Aug 25, 2025

CI is failing now, seems like a typo in the merge resolution.

Thanks, fixed.

@vbraun vbraun merged commit c1a62af into sagemath:develop Aug 27, 2025
21 of 24 checks passed
@antonio-rojas
Copy link
Contributor Author

Follow-up at #40704

@vbraun
Copy link
Member

vbraun commented Aug 31, 2025

The test failures are now reported as

sage-runtests --long --warn-long 30.0 --random-seed=0 src/sage/misc/sagedoc.py  # 1 doctest failed

which isn't a command that I can easily run from the repo root. IMHO thats a definite regression, it is much clearer to output a command that you can just run to re-run the test

@antonio-rojas
Copy link
Contributor Author

The test failures are now reported as

sage-runtests --long --warn-long 30.0 --random-seed=0 src/sage/misc/sagedoc.py  # 1 doctest failed

which isn't a command that I can easily run from the repo root. IMHO thats a definite regression, it is much clearer to output a command that you can just run to re-run the test

That's easy to change, but note that, depending on your PATH, sage -t may stop working after #39030 (if the first sage found is the compiled one). Would printing the full path to sage-runtests be OK?

@vbraun
Copy link
Member

vbraun commented Aug 31, 2025

a full path would be ok with me, I don't understand why we need a different command though (but then thats arguably less important)

@tobiasdiez
Copy link
Contributor

What about the relative path from the root? So src/bin/sage-runtests ? I think it's most common to execute stuff from the root, at least CI is doing this.

@antonio-rojas
Copy link
Contributor Author

#40746

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.

3 participants