Skip to content

Migrate "long" ci tests to Meson #40158

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

Draft
wants to merge 15 commits into
base: develop
Choose a base branch
from

Conversation

tobiasdiez
Copy link
Contributor

Fixes #40120.

Sage-the-distro is still tested using ci-incremental.

📝 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

@user202729
Copy link
Contributor

Technically this "works" too (if it works properly). Let's see.

@@ -42,6 +45,11 @@ jobs:
- os: windows
python: '3.13'
tests: 'all'
exclude:
# Exclude 'all' since we already have 'long'
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe it's time to consider having a more descriptive name. new (both short and long), short, long (this should really be all, but that's doubly confusing because all currently means all short)?

@user202729
Copy link
Contributor

Sage-the-distro is still tested using ci-incremental.

(if you mean ci-linux-incremental) technically this is false since the test is only run on a pull request in the (relatively rare) case where some package has changed.

@@ -373,15 +373,3 @@ exclude =
sage/combinat/designs/database.py
sage/graphs/graph_plot.py
sage/misc/sagedoc.py

[coverage:run]
Copy link
Contributor

Choose a reason for hiding this comment

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

is there anything else that still uses tox?

@user202729
Copy link
Contributor

the put(result, False) is strange, and it was added all the way back in 9b1c246 .

Maybe try changing it to .put(result, block=True, timeout=10)? That way at least it tolerates the reader is slow, but also appropriately complain after a while if the reader malfunction.

@tobiasdiez
Copy link
Contributor Author

the put(result, False) is strange, and it was added all the way back in 9b1c246 .

Maybe try changing it to .put(result, block=True, timeout=10)? That way at least it tolerates the reader is slow, but also appropriately complain after a while if the reader malfunction.

Do you understand what's going on here? The queue has a max limit of 1, because it is supposed to hold only one doctest result. I don't see why it would try to put more than one result in there. The code also doesn't seem to reuse the queue across workers/doctests.

@user202729
Copy link
Contributor

Do you understand what's going on here? The queue has a max limit of 1, because it is supposed to hold only one doctest result. I don't see why it would try to put more than one result in there. The code also doesn't seem to reuse the queue across workers/doctests.

no idea either, but some StackOverflow post claims this may be caused when the object being passed around is too large (I can't reproduce this on my side though). In that case it sounds like blocking=True might help.

Worst case we can add some debugging there (print out what's the current length), modulo race conditions.

@user202729
Copy link
Contributor

user202729 commented May 25, 2025

sage -t --long --warn-long 30.0 --random-seed=308424213503200308932561197805792472951 src/sage/dynamics/arithmetic_dynamics/projective_ds.py
Error: sage: P1.<x,y> = ProjectiveSpace(QQ,1) ## line 169 ##
sage: DynamicalSystem_projective([y, 2*x]) ## line 170 ##
Dynamical System of Projective Space of dimension 1 over Rational Field
  Defn: Defined on coordinates by sending (x : y) to
        (y : 2*x)
sage: R.<t> = QQ[] ## line 178 ##
sage: DynamicalSystem_projective(t^2 - 3) ## line 179 ##
Dynamical System of Projective Space of dimension 1 over Rational Field
  Defn: Defined on coordinates by sending (X : Y) to
        (X^2 - 3*Y^2 : Y^2)
sage: DynamicalSystem_projective(1/t^2) ## line 183 ##
Dynamical System of Projective Space of dimension 1 over Rational Field
  Defn: Defined on coordinates by sending (X : Y) to
        (Y^2 : X^2)
sage: R.<x> = PolynomialRing(QQ,1) ## line 190 ##
sage: DynamicalSystem_projective(x^2, names=['a','b']) ## line 191 ##
Dynamical System of Projective Space of dimension 1 over Rational Field
  Defn: Defined on coordinates by sending (a : b) to
        (a^2 : b^2)
sage: x,y = var('x,y')                             
Process DocTestWorker-1129:
Traceback (most recent call last):
  File "/usr/share/miniconda/envs/sage-dev/lib/python3.12/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "/usr/share/miniconda/envs/sage-dev/lib/python3.12/site-packages/sage/doctest/forker.py", line 2301, in run
    task(self.options, self.outtmpfile, msgpipe, self.result_queue,
  File "/usr/share/miniconda/envs/sage-dev/lib/python3.12/site-packages/sage/doctest/forker.py", line 2637, in __call__
    result_queue.put(result, block=True, timeout=10)
  File "/usr/share/miniconda/envs/sage-dev/lib/python3.12/multiprocessing/queues.py", line 90, in put
    raise Full
queue.Full

it's not helping either. I'll see if I can reproduce the issue locally…

edit: I cannot. debugging with #40161

edit: feels like a weird bug in multiprocessing.Queue to me. By debugging I determine that qsize() returns 1, yet get() fails.

Now continue debugging.

@user202729
Copy link
Contributor

sanity check, is test-new currently running long tests? (it's supposed to be.)

Copy link

github-actions bot commented Jun 4, 2025

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

@tobiasdiez tobiasdiez marked this pull request as draft June 14, 2025 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test failure in test-long etc. because of singular
2 participants