Skip to content

Conversation

collares
Copy link
Contributor

@collares collares commented Sep 27, 2025

These two sage_docbuild tests use a relative path, requiring sage -t --all to be run from a specific directory. This was never needed before, so perhaps it was unintended.

Copy link

github-actions bot commented Sep 27, 2025

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

@tobiasdiez
Copy link
Contributor

How do you run into this issue? I.e. why do you have the sage_doctest folder around but not src/doc?

@collares
Copy link
Contributor Author

collares commented Sep 28, 2025

@tobiasdiez Yes, you're right. The problem with the new tests is something else: Before, the paths in the relevant tests were prefixed by Path(os.environ['SAGE_DOC_SRC']), while now they use relative paths such as src/doc. Therefore, running the full testsuite (sage -t --all) now needs to be done in a specific working directory, which wasn't the case before. Is that intended?

@collares collares changed the title Restore some sage_spkg optional tags in sage_docbuild Use SAGE_DOC_SRC instead of relative path in docbuilder test Sep 28, 2025
@tobiasdiez
Copy link
Contributor

@tobiasdiez Yes, you're right. The problem with the new tests is something else: Before, the paths in the relevant tests were prefixed by Path(os.environ['SAGE_DOC_SRC']), while now they use relative paths such as src/doc. Therefore, running the full testsuite (sage -t --all) now needs to be done in a specific working directory, which wasn't the case before. Is that intended?

No this was indeed not intended. Thanks for bringing this up and providing a fix! One small suggestion from my side, otherwise it's good to go.

@collares
Copy link
Contributor Author

Fixed. Thanks for the review!

Copy link
Contributor

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

CI issues seem to be unrelated.

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.

2 participants