Skip to content

Conversation

jamesbraza
Copy link
Collaborator

  • test_can_read_normal_pdf_reader was a subset of test_fileio_reader_pdf
  • Named some pytest fixtures so pylint can not report name collisions

@jamesbraza jamesbraza self-assigned this Jul 23, 2025
@Copilot Copilot AI review requested due to automatic review settings July 23, 2025 04:06
@jamesbraza jamesbraza added the bug Something isn't working label Jul 23, 2025
@dosubot dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Jul 23, 2025
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR cleans up test code by removing a redundant test function and addressing pylint warnings about fixture naming. The removed test test_can_read_normal_pdf_reader was identified as being a subset of existing test coverage in test_fileio_reader_pdf.

  • Removed duplicate test function test_can_read_normal_pdf_reader
  • Added explicit names to pytest fixtures to prevent pylint name collision warnings
  • Minor code reorganization in test_fileio_reader_pdf

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/test_paperqa.py Removed redundant test function, renamed fixture function, and minor reordering
tests/conftest.py Added explicit names to pytest fixtures and renamed fixture functions
Comments suppressed due to low confidence (4)

tests/test_paperqa.py:70

  • [nitpick] The function name 'fixture_docs_fixture' is redundant and unclear. Consider renaming to 'create_docs' or 'docs_with_paper' to better describe what the fixture provides.
async def fixture_docs_fixture(stub_data_dir: Path) -> Docs:

tests/conftest.py:68

  • [nitpick] The function name 'fixture_tmp_path_cleanup' is redundant. Consider renaming to 'tmp_path_with_cleanup' or 'auto_cleanup_tmp_path' to better describe the fixture's behavior.
def fixture_tmp_path_cleanup(tmp_path: Path) -> Iterator[Path]:

tests/conftest.py:76

  • [nitpick] The function name 'fixture_agent_home_dir' is redundant. Consider renaming to 'setup_agent_home_dir' or 'agent_home_with_env' to better describe the fixture's purpose.
def fixture_agent_home_dir(

tests/conftest.py:85

  • [nitpick] The function name 'fixture_agent_index_dir' is redundant. Consider renaming to 'get_agent_index_dir' or 'agent_index_path' to better describe what the fixture returns.
def fixture_agent_index_dir(agent_home_dir: Path) -> Path:

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jul 23, 2025
@jamesbraza jamesbraza merged commit f9da499 into main Jul 23, 2025
6 checks passed
@jamesbraza jamesbraza deleted the removed-extra-test branch July 23, 2025 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants