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

Fix the type annotation for sphinx.testing.fixtures.rootdir #12764

Merged
merged 2 commits into from
Aug 11, 2024

Conversation

ftnext
Copy link
Contributor

@ftnext ftnext commented Aug 11, 2024

Subject: Tweak rootdir fixture's type hint

Feature or Bugfix

  • Refactoring

Purpose

https://www.sphinx-doc.org/en/master/extdev/testing.html#usage

If you want to know more detailed usage, please refer to tests/conftest.py` and other test_*.py files under the tests/ directory.

In tests/conftest.py, I noticed that rootdir returns pathlib.Path instead of str.

sphinx/tests/conftest.py

Lines 43 to 45 in 043750e

@pytest.fixture(scope='session')
def rootdir() -> Path:
return Path(__file__).parent.resolve() / 'roots'

rootdir in sphinx/testing/fixture.py is type-hinted as returning str

@pytest.fixture(scope='session')
def rootdir() -> str | None:
return None

but rootdir is treated as pathlib.Path in app_params fixture.

if rootdir and not srcdir.exists():
testroot_path = rootdir / ('test-' + testroot)
shutil.copytree(testroot_path, srcdir)

  • pathlib.Path / str: works!
  • str / str: not work

Detail

(See Purpose. That's all)

Relates

  • N/A

@AA-Turner AA-Turner changed the title tweak: rootdir expects pathlib.Path Fix the type annotation for sphinx.testing.fixtures.rootdir Aug 11, 2024
@AA-Turner AA-Turner merged commit df75a67 into sphinx-doc:master Aug 11, 2024
20 checks passed
@ftnext ftnext deleted the tweak-rootdir-fixture-type-hint branch August 12, 2024 04:57
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2024
@AA-Turner AA-Turner added this to the 8.1.x milestone Oct 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants