Skip to content

Rename module-global test methods #39208

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 11 commits into from
Jul 6, 2025

Conversation

tobiasdiez
Copy link
Contributor

Pytest automatically identifies and runs all methods prefixed with test_, following the standard convention for denoting tests. In the Sage codebase, there are numerous methods with the test_ prefix. While many of these are indeed intended for testing purposes, they do not conform to Pytest's requirements for valid tests. To address this, we are temporarily renaming these methods, with the goal of eventually converting them into proper Pytest tests in the future.

📝 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

Pytest automatically identifies and runs all methods prefixed with `test_`, following the standard convention for denoting tests. In the Sage codebase, there are numerous methods with the `test_` prefix. While many of these are indeed intended for testing purposes, they do not conform to Pytest's requirements for valid tests. To address this, we are temporarily renaming these methods, with the goal of eventually converting them into proper Pytest tests in the future.
Copy link

github-actions bot commented Dec 27, 2024

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

@user202729
Copy link
Contributor

How about pytest.mark.skip? This risks being a breaking change.

@tobiasdiez
Copy link
Contributor Author

How about pytest.mark.skip? This risks being a breaking change.

That might work as well, but then they would show up as "skipped tests" in pytest - which is also not really nice.

@user202729
Copy link
Contributor

I just come across a solution that doesn't involve changing the name https://stackoverflow.com/a/71164113 (though not sure if it's worth)

@tobiasdiez
Copy link
Contributor Author

I just come across a solution that doesn't involve changing the name https://stackoverflow.com/a/71164113 (though not sure if it's worth)

Thanks! If someone dislikes the renames here, this would indeed be a nice workaround.

@tobiasdiez tobiasdiez requested a review from dimpase June 30, 2025 14:10
@dimpase
Copy link
Member

dimpase commented Jul 1, 2025

that's fine with me

@vbraun vbraun merged commit 13a92c6 into sagemath:develop Jul 6, 2025
25 checks passed
@tobiasdiez tobiasdiez deleted the pytest_rename_check branch July 6, 2025 15:56
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