rereading/arcalibre
15
85
Fork 1
Code Issues 19 Pull requests 3 Packages 1 Wiki Activity

Migrate existing unit tests to pytest. #20

Merged
cgranade merged 31 commits from cgranade/pytest into master 2025-12-28 10:52:20 +01:00
Conversation 3 Commits 31 Files changed 139 +1905 -2520
cgranade commented 2025-12-25 01:18:26 +01:00
Owner
Copy link

The original Calibre package used an ad-hoc test discovery framework, wherein each module under test defines a find_tests function that is then called by an eventual test harness command in make.py. This causes issues such as #19, wherein some modes of test failure can propagate to errors in the test harness itself, preventing any unit tests from being run.

This PR addresses #19 by moving unit tests into a separate folder that depends on the calibre package and on pytest. This in turn requires removing the pre-existing hack wherein the run-local script would add attributes to the sys module, when would then be used by other modules such as calibre.constants or the extensions meta-loader. Modifying sys in place prevented the import of the calibre package from a unit-testing context, and makes it more difficult to extract subpackages and modules into standalone wheels.

This PR does not attempt to get failing unit tests to pass, beyond very basic fixes — rather, the focus is on getting unit tests to run at all, so that regressions can be found when refactoring other parts of Calibre into the Arcalibre codebase.

This PR is marked as WIP until all tests have been migrated so that an accurate picture can be reached as to how many pre-existing tests currently fail.

The original Calibre package used an ad-hoc test discovery framework, wherein each module under test defines a `find_tests` function that is then called by an eventual test harness command in `make.py`. This causes issues such as #19, wherein some modes of test failure can propagate to errors in the test harness itself, preventing any unit tests from being run. This PR addresses #19 by moving unit tests into a separate folder that depends on the `calibre` package and on `pytest`. This in turn requires removing the pre-existing hack wherein the `run-local` script would add attributes to the `sys` module, when would then be used by other modules such as `calibre.constants` or the extensions meta-loader. Modifying `sys` in place prevented the import of the `calibre` package from a unit-testing context, and makes it more difficult to extract subpackages and modules into standalone wheels. This PR does not attempt to get failing unit tests to pass, beyond very basic fixes — rather, the focus is on getting unit tests to run at *all*, so that regressions can be found when refactoring other parts of Calibre into the Arcalibre codebase. This PR is marked as WIP until all tests have been migrated so that an accurate picture can be reached as to how many pre-existing tests currently fail.
cgranade commented 2025-12-27 06:40:47 +01:00
Author
Owner
Copy link

Many of the tests remain broken, and I'm sure I introduced some bugs along the way, but here's the final score for having moved all Python tests into a pytest suite:

  • 134 ty diagnostics
  • 26 ruff diagnostics that can't be fixed with --fix
  • 91 failed tests, 276 passed, 15 skipped, 54 warnings, and 4 errors
  • Tests take 41 seconds to run on my laptop
Many of the tests remain broken, and I'm sure I introduced some bugs along the way, but here's the final score for having moved all Python tests into a pytest suite: - 134 `ty` diagnostics - 26 `ruff` diagnostics that can't be fixed with `--fix` - 91 failed tests, 276 passed, 15 skipped, 54 warnings, and 4 errors - Tests take 41 seconds to run on my laptop
cgranade changed title from WIP: Migrate existing unit tests to pytest. to Migrate existing unit tests to pytest. 2025-12-27 06:44:52 +01:00
cgranade commented 2025-12-27 09:08:19 +01:00
Author
Owner
Copy link

Fixes #19.

Fixes #19.
cgranade commented 2025-12-28 10:52:11 +01:00
Author
Owner
Copy link

Since multiple people have given this a try and confirmed that it works locally (in that it runs and returns a list of failed tests), I'm pretty confident with merging this in now to make it easier to test other PRs. Thank you so much to @SnoopJ and @cthos!

Since multiple people have given this a try and confirmed that it works locally (in that it runs and returns a list of failed tests), I'm pretty confident with merging this in now to make it easier to test other PRs. Thank you so much to @SnoopJ and @cthos!
cgranade merged commit eb8e923173 into master 2025-12-28 10:52:20 +01:00
cgranade deleted branch cgranade/pytest 2025-12-28 10:52:39 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
area/ai-removal
Affects the removal of AI antifeatures

  
area/build
Affects build stabilization

  
area/meta
Affects things about the project itself, such as governance issues or contribution policies

  
bug
Something is not working

  
contribution welcome
Contributions are very welcome, get started here

  
duplicate
This issue or pull request already exists

  
good first issue
Interested in contributing? Get started here.

  
help wanted
Need some help

  
invalid
Something is wrong

  
question
More information is needed

  
upstream
Related to an upstream repository, already reported there

No labels
area/ai-removal
area/build
area/meta
bug
contribution welcome
duplicate
good first issue
help wanted
invalid
question
upstream
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
rereading/arcalibre!20
No description provided.