Skip to content

Conversation

@JoshCLWren
Copy link
Owner

Summary

  • Update test coverage threshold from 95% to 96% in pyproject.toml
  • Fix test suite crash with coverage enabled by adding proper thread cleanup to ImageWorker
  • Add ImageWorker.stop() method to signal threads to exit and wait for cleanup
  • Call worker.stop() in ComicViewer._quit() and add del safety net
  • Add image_worker fixture to conftest.py for proper test teardown
  • Update documentation (AGENTS.md, tasks.md, TESTING.md) to reflect 96% coverage milestone
  • Add coverage files to .gitignore

Testing

All 413 tests pass with 96.59% coverage:

  • Ran full test suite with coverage enabled: uv run pytest --cov=cdisplayagain --cov-report=term-missing
  • Verified tests complete successfully without threading crashes

Changes

  • cdisplayagain.py: Add thread cleanup and del method
  • tests/conftest.py: Add image_worker fixture for proper teardown
  • pyproject.toml: Update coverage threshold to 96%
  • AGENTS.md: Update coverage milestone to 96%
  • tasks.md: Update coverage milestone to 96%
  • docs/archive/TESTING.md: Update coverage testing command
  • .gitignore: Add coverage files

Add proper thread cleanup to ImageWorker.stop() with stop signals and queue
timeout handling. Call worker.stop() in ComicViewer._quit() and add __del__
safety net. Add image_worker fixture to conftest.py for proper test teardown.

Update test coverage threshold to 96% in pyproject.toml and reflect milestone
in AGENTS.md, tasks.md, and TESTING.md documentation.
@JoshCLWren JoshCLWren force-pushed the move-test-coverage-marker branch from c0eb93d to de82eb4 Compare December 27, 2025 22:33
Remove autouse fixture and simplify cleanup. Fix _run() exception handling
to break immediately on any exception instead of checking priority value.
This prevents workers from trying to access destroyed app state during teardown.
Add try/finally blocks to ensure ImageWorker.stop() is called before
Tk root destruction in all tests that create workers manually.
This prevents threads stuck in queue.get() from causing CI crashes.

- tests/test_parallel_workers.py: Add cleanup to 10 tests
- tests/test_threading.py: Add cleanup to 4 tests
- tests/test_benchmark_parallel.py: Add cleanup to 2 tests
Replace try/finally blocks with Pythonic context manager for ImageWorker.
This reduces boilerplate and makes cleanup automatic and explicit.

- Add __enter__ and __exit__ methods to ImageWorker
- Update 16 tests to use 'with ImageWorker() as worker:' pattern
- Remove 57 lines of try/finally boilerplate
- Add assert app is not None after null check to satisfy pyright
- Add source=None check in worker loop for graceful shutdown
- Add test_worker_handles_none_source_gracefully to cover shutdown path
- Coverage now at 96.00% (target reached)
- All linting passes with 0 type checking errors
@codecov
Copy link

codecov bot commented Dec 27, 2025

Codecov Report

❌ Patch coverage is 96.61017% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cdisplayagain.py 96.61% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@JoshCLWren JoshCLWren force-pushed the move-test-coverage-marker branch 3 times, most recently from af85c83 to 1c8d699 Compare December 28, 2025 00:13
@JoshCLWren JoshCLWren force-pushed the move-test-coverage-marker branch 2 times, most recently from 6a03362 to b3e94fd Compare December 28, 2025 03:11
- Add tests for request_page early return when stopped
- Add tests for request_page when app is None
- Add tests for queue.Full exception handling
- Add tests for context manager behavior
- Add tests for cleanup() method
- Add test for __del__ method calling cleanup
- Add test for after_idle exception handling
- Add test for general exception in worker
- Add test for queue.Full exception in stop()
- Add test for thread.join exception in stop()
- Add test for worker stopping mid-processing
Extracted stop-check into _should_stop() method to enable reliable test coverage
of thread interruption points without race conditions. Added 5 tests covering stop
behavior at different processing stages. Fixed Tk cleanup in conftest.
@JoshCLWren
Copy link
Owner Author

John Cupitt | John Cupitt | 6:19 AM (2 hours ago) |   |   -- | -- | -- | -- | -- John Cupitt

@jcupitt commented on this pull request.

Other:

  • we need a line in the changelog, and please credit yourself for doing this nice work
  • generate_type_stubs.py needs to be set executable
  • does the type stub generator need to be in pyvips/? it's something for maintainers, so I'd be inclined to put it in examples/. The doc generator is in pyvips/ because it's used for help() output
  • perhaps we could run mypy on the demo scripts in examples/ as part of CI? but it'll need eg. a hint for image * [1, 2, 3] first

In pyvips/generate_type_stubs.py:

> +    def set_kill(self, kill: bool) -> None: ...
+    def copy(self, **kwargs: object) -> Image: ...
+    def tolist(self) -> List[List[float]]: ...
+    # numpy is optional dependency - use TYPE_CHECKING guard
+    def __array__(self, dtype: Optional[str] = None, copy: Optional[bool] = None) -> object: ...
+    def numpy(self, dtype: Optional[str] = None) -> object: ...
+
+    # Dynamically generated operations
+'''
+
+    stub += generate_all_image_operations()
+
+    stub += """
+
+    # Operator overloads
+    def __add__(self, other: Union[Image, float, int]) -> Image: ...

You can have arrays of float and int too, eg. a + [1, 2, 3] adds 1 to band 0, 2 to band 1, 3 to band 2.


In pyvips/generate_type_stubs.py:

> +    def __or__(self, other: Union[Image, float, int]) -> Image: ...
+    def __ror__(self, other: Union[Image, float, int]) -> Image: ...
+    def __xor__(self, other: Union[Image, float, int]) -> Image: ...
+    def __rxor__(self, other: Union[Image, float, int]) -> Image: ...
+    def __eq__(self, other: object) -> bool: ...
+    def __ne__(self, other: object) -> bool: ...
+    def __gt__(self, other: Union[Image, float, int]) -> Image: ...
+    def __ge__(self, other: Union[Image, float, int]) -> Image: ...
+    def __lt__(self, other: Union[Image, float, int]) -> Image: ...
+    def __le__(self, other: Union[Image, float, int]) -> Image: ...
+
+    def __getitem__(self, arg: Union[int, slice, List[int], List[bool]]) -> Image: ...
+    def __call__(self, x: int, y: int) -> List[float]: ...
+    def __repr__(self) -> str: ...
+
+

There are a set of hand-written bindings too, perhaps some of them could have type hints?

Easy: image.floor() etc.

Medium: bandjoin, bandsplit, composite etc.

Horrible: ifthenelse.


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.

John Cupitt

6:19 AM (2 hours ago)

to libvips/pyvips, me, Mention

@jcupitt commented on this pull request.

Other:

we need a line in the changelog, and please credit yourself for doing this nice work
generate_type_stubs.py needs to be set executable
does the type stub generator need to be in pyvips/? it's something for maintainers, so I'd be inclined to put it in examples/. The doc generator is in pyvips/ because it's used for help() output
perhaps we could run mypy on the demo scripts in examples/ as part of CI? but it'll need eg. a hint for image * [1, 2, 3] first

In pyvips/generate_type_stubs.py:

  • def set_kill(self, kill: bool) -> None: ...
  • def copy(self, **kwargs: object) -> Image: ...
  • def tolist(self) -> List[List[float]]: ...
  • numpy is optional dependency - use TYPE_CHECKING guard

  • def array(self, dtype: Optional[str] = None, copy: Optional[bool] = None) -> object: ...
  • def numpy(self, dtype: Optional[str] = None) -> object: ...
  • Dynamically generated operations

+'''
+

  • stub += generate_all_image_operations()
  • stub += """
  • Operator overloads

  • def add(self, other: Union[Image, float, int]) -> Image: ...

You can have arrays of float and int too, eg. a + [1, 2, 3] adds 1 to band 0, 2 to band 1, 3 to band 2.

In pyvips/generate_type_stubs.py:

  • def or(self, other: Union[Image, float, int]) -> Image: ...
  • def ror(self, other: Union[Image, float, int]) -> Image: ...
  • def xor(self, other: Union[Image, float, int]) -> Image: ...
  • def rxor(self, other: Union[Image, float, int]) -> Image: ...
  • def eq(self, other: object) -> bool: ...
  • def ne(self, other: object) -> bool: ...
  • def gt(self, other: Union[Image, float, int]) -> Image: ...
  • def ge(self, other: Union[Image, float, int]) -> Image: ...
  • def lt(self, other: Union[Image, float, int]) -> Image: ...
  • def le(self, other: Union[Image, float, int]) -> Image: ...
  • def getitem(self, arg: Union[int, slice, List[int], List[bool]]) -> Image: ...
  • def call(self, x: int, y: int) -> List[float]: ...
  • def repr(self) -> str: ...

There are a set of hand-written bindings too, perhaps some of them could have type hints?

Easy: image.floor() etc.

Medium: bandjoin, bandsplit, composite etc.

Horrible: ifthenelse.


Reply to this email directly, view it on GitHub, or unsubscribe.
You are receiving this because you were mentioned.

@JoshCLWren JoshCLWren merged commit 6f21c60 into main Dec 28, 2025
3 checks passed
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