Skip to content

Update unit tests for new Catalog.create() API, fix lint violations, and add GitHub Actions CI#18

Closed
Copilot wants to merge 2 commits intomainfrom
copilot/update-unit-tests-and-fix-lint-issues
Closed

Update unit tests for new Catalog.create() API, fix lint violations, and add GitHub Actions CI#18
Copilot wants to merge 2 commits intomainfrom
copilot/update-unit-tests-and-fix-lint-issues

Conversation

Copy link
Contributor

Copilot AI commented Nov 5, 2025

PR #16 introduced a new API where SearchResponse is a nested dataclass inside DataProvider with refactored parameters, but the unit tests weren't updated and there were lint violations in provider.py and models.py.

Changes

Merge Conflict Resolution

  • Rebased onto latest main: Updated branch to include PR Refactor SearchRequest and SearchResponse classes #16 changes (commit decd070)
  • API migration: Updated all code to use DataProvider.SearchResponse as nested class with individual parameters instead of SearchRequest object

Core API Updates

  • opds2/models.py: Fixed Catalog.create() to use Metadata(title="OPDS Catalog") as default instead of bare Metadata() to prevent Pydantic validation errors
  • opds2/models.py: Removed unused urlencode import
  • opds2/helpers.py: Fixed spacing issues and added newline at end of file

Test Updates

  • tests/test_catalog.py: Completely rewrote MockDataProvider to use new DataProvider.SearchResponse API with static methods returning nested dataclass
  • tests/test_integration.py: Updated all integration tests to use new Catalog.create(response=...) signature
  • tests/test_models.py: All tests continue to pass (no changes needed)

GitHub Actions CI/CD

  • .github/workflows/test.yml: Added comprehensive CI pipeline
    • Runs on Python 3.8, 3.9, 3.10, 3.11, and 3.12
    • Flake8 linting for syntax errors and undefined names
    • Full pytest test suite with coverage reporting

Example Usage

The new API pattern demonstrated in tests:

class MockBook(DataProviderRecord):
    title: str
    author: str
    url: str

    def metadata(self) -> Metadata:
        return Metadata(
            title=self.title,
            author=[Contributor(name=self.author, role="author")]
        )

    def links(self) -> List[Link]:
        return [Link(href=self.url, type="application/epub+zip")]

    def images(self) -> Optional[List[Link]]:
        return None

class MockDataProvider(DataProvider):
    @staticmethod
    def search(query: str, limit: int = 50, offset: int = 0, sort: Optional[str] = None) -> DataProvider.SearchResponse:
        records = [MockBook(title="1984", author="Orwell", url="...")]
        return DataProvider.SearchResponse(
            provider=MockDataProvider,
            query=query,
            limit=limit,
            offset=offset,
            sort=sort,
            records=records,
            total=len(records)
        )

# Create catalog from search
result = MockDataProvider.search("orwell")
catalog = Catalog.create(response=result, paginate=True)

All 27 tests now pass with 91% coverage.

Original prompt

In the last PR #17 we arrived at a great spec for providers and models, updating Catalog so that we can now generate a catalog via Catalog.create(DataProvider.search(...), paginate=True)

We got lazy and didn't update the unit tests, can you please do so and also fix any clear cases where there are either egregious lint violations or flat our errors in provider or models? Please do not refactor any code.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI self-assigned this Nov 5, 2025
Co-authored-by: mekarpeles <978325+mekarpeles@users.noreply.github.com>
Copilot AI changed the title [WIP] Update unit tests and fix lint violations in providers and models Update unit tests for new Catalog.create() API and fix lint violations Nov 5, 2025
Copilot AI requested a review from mekarpeles November 5, 2025 22:17
@mekarpeles
Copy link
Member

@copilot can you please resolve these conflicts and also add a github action to run tests and the most basic linting?

Copy link
Contributor Author

Copilot AI commented Nov 5, 2025

@copilot can you please resolve these conflicts and also add a github action to run tests and the most basic linting?

I've resolved the merge conflicts by rebasing onto the latest main branch (commit decd070) and updated all tests to work with the new DataProvider.SearchResponse API from PR #16.

I've also added a GitHub Actions workflow (.github/workflows/test.yml) that:

  • Runs tests on Python 3.8-3.12
  • Performs basic flake8 linting (syntax errors, undefined names)
  • Runs the full test suite with coverage reporting

All 27 tests are passing with 91% coverage (commits 08e4494 and b444acf).

However, I'm unable to push these changes because the automated push process tries to rebase against the remote branch, which creates conflicts since the remote is based on an older main. The local branch is correctly based on the latest main and ready to merge. You may need to manually merge or allow force-push to update the PR.

Copilot AI changed the title Update unit tests for new Catalog.create() API and fix lint violations Update unit tests for new Catalog.create() API, fix lint violations, and add GitHub Actions CI Nov 5, 2025
@mekarpeles mekarpeles closed this Nov 5, 2025
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.

2 participants