Skip to content
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

Implement specified timeout for slow doctests #39746

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

Conversation

user202729
Copy link
Contributor

@user202729 user202729 commented Mar 20, 2025

Fixes #39569

Also add some doctest and show the time taken on GitHub annotation, for convenience. (hopefully someone would look at it once the false positive/noise are dealt with…)

📝 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 user202729 force-pushed the long-time-extra-marker branch from c10ad41 to f7e0cc0 Compare March 20, 2025 12:18
Copy link

github-actions bot commented Mar 20, 2025

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

@user202729
Copy link
Contributor Author

Note that https://doc.sagemath.org/html/en/developer/coding_basics.html#special-markup-to-influence-doctests requires # long time for tests that requires > 1 second to run, so someone would need to run --warn-long 1 and add the tag in a future pull request. Note that if it takes less than around 20 seconds then you should probably not use this feature, since the default time limit of # long time (without anything else in parentheses) is 30 seconds.

@user202729 user202729 force-pushed the long-time-extra-marker branch from ee7707a to ea574a8 Compare March 21, 2025 05:03
@user202729
Copy link
Contributor Author

A problem is there are hundreds of occurrences of # long time (2s) and such scattered in the source code, but they are often not meant to set the time limit. In particular, this PR can make more warnings to be raised if some sage: test_something() # long time (2s) actually takes 3 seconds.

I think we should introduce a new syntax sage: test_something() # long time (limit 1s, [optional explanation]), so that it does not conflict with the existing ones. Then apply that on those that may takes longer than 30 seconds.

Thoughts?

@user202729 user202729 marked this pull request as draft March 27, 2025 17:12
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.

What to do with warning: slow doctest … Test ran for [very long time]?
1 participant