[2/5] Type-check assertions #14

dandavison · 2025-07-12T20:22:27Z

This is PR 2/5 in a stack.

Update the library for making type-check failure assertions so that it supports making assertions for multiple type checkers.

The main point of this PR is the changes to the library for type-check assertions test_type_errors.py

dandavison · 2025-07-14T13:09:16Z

pyproject.toml

@@ -39,8 +39,8 @@ packages = ["src/nexusrpc"]

 [tool.poe.tasks]
 lint = [
-  {cmd = "uv run basedpyright src"},
-  {cmd = "uv run pyright src"},


The test suite passes type-checking after this PR.

cretz

Not applying much scrutiny to tests here, but did mention in a previous PR that reportDeprecated is noisy on our PRs here

dandavison · 2025-07-14T15:11:08Z

Thanks!

Not applying much scrutiny to tests here,

You might want to look at test_type_errors.py -- it's not a test, but rather a framework for making assertions about type checker error diagnostics.

cretz · 2025-07-14T16:10:35Z

You might want to look at test_type_errors.py -- it's not a test, but rather a framework for making assertions about type checker error diagnostics.

Will look...

cretz · 2025-07-14T16:09:43Z

tests/test_type_errors.py

+# This test is disabled since we currently have no way to be able to
+# assert-type-error-mypy on a line with a `type: ignore`.


Can't say I understand this, but if this is dead code, can we remove it or open an issue to follow up on it?

Deleted. The issue is that:

We would like to be able to make an assertion that a line produces a certain type error

We would also like the test code to pass type checking

For pyright, the current solution is to place the type: ignore on the line in question but instruct pyright to ignore it when performing the type error assertion check.

For mypy we have no such solution

There are probably other solutions, e.g. entirely excluding such files from our type checking in CI. I'll look into that: I think that may work because the type assertion checker does check that no there are no type errors other than the expected ones. But let's merge this as-is if that's OK, i.e. with just pyright support.

cretz · 2025-07-14T16:10:03Z

tests/test_type_errors.py

+        metafunc.parametrize("test_file", files_with_assertions, ids=lambda f: f.name)
+
+
+@pytest.mark.skipif(platform.system() == "Windows", reason="TODO: broken on Windows")


Can we open an issue to address this TODO so we don't lose track?

Yes good call, done #19. There should be no technical obstacle there; I just failed to solve it within the time I had allotted to the task.

cretz · 2025-07-14T16:11:53Z

tests/test_type_errors.py

+import pytest
+
+
+def pytest_generate_tests(metafunc: pytest.Metafunc) -> None:


I bit confused here. How is this used? It seems to look for certain function names but ignores them if they are in this file but I can find them nowhere else? Is this code not leveraged in this PR?

It's a pytest hook https://docs.pytest.org/en/stable/how-to/parametrize.html#pytest-generate-tests. Here we're using it to dynamically generate a test for every test file that contains type error assertion comments.

It generates a test like this (we only have those comments in one file at the moment):

tests/test_type_errors.py::test_type_errors_pyright[test_type_checking_sync_operation.py] PASSED

AI: This commit should have tag name type-assertions and should be force-pushed to a branch of that name as necessary.

dandavison force-pushed the typing-cleanup branch 2 times, most recently from 31c31f3 to 58d2a77 Compare July 12, 2025 20:38

dandavison force-pushed the type-assertions branch from 775285c to d164ae4 Compare July 12, 2025 20:38

dandavison changed the title ~~Type assertions~~ [2/4] Type-check assertions Jul 12, 2025

dandavison force-pushed the type-assertions branch 5 times, most recently from 912ea11 to 97376d2 Compare July 12, 2025 22:08

dandavison force-pushed the typing-cleanup branch from 58d2a77 to 6a5080c Compare July 12, 2025 22:28

dandavison force-pushed the type-assertions branch 3 times, most recently from 25aebe9 to 12ceb41 Compare July 13, 2025 18:50

dandavison force-pushed the typing-cleanup branch from 6a5080c to daee4f6 Compare July 14, 2025 13:04

dandavison force-pushed the type-assertions branch from 12ceb41 to 1dd575e Compare July 14, 2025 13:04

dandavison commented Jul 14, 2025

View reviewed changes

dandavison changed the title ~~[2/4] Type-check assertions~~ [2/5] Type-check assertions Jul 14, 2025

cretz approved these changes Jul 14, 2025

View reviewed changes

dandavison force-pushed the typing-cleanup branch from daee4f6 to 635a365 Compare July 14, 2025 15:01

dandavison force-pushed the type-assertions branch from 1dd575e to 1d43557 Compare July 14, 2025 15:02

dandavison force-pushed the typing-cleanup branch from 635a365 to 6e68f3f Compare July 14, 2025 15:08

dandavison force-pushed the type-assertions branch from 1d43557 to 76241b2 Compare July 14, 2025 15:08

Base automatically changed from typing-cleanup to main July 14, 2025 15:54

dandavison added 4 commits July 14, 2025 11:54

Support type assertions for multiple type checkers

3ed760b

Skip type checking on Windows

49ed186

Delete failing tests of callable instances

0df0b57

Run basedpyright on all code

c666436

dandavison force-pushed the type-assertions branch from 76241b2 to 6e7bdd3 Compare July 14, 2025 15:56

cretz reviewed Jul 14, 2025

View reviewed changes

Rename type errors test

f6d4657

AI: This commit should have tag name type-assertions and should be force-pushed to a branch of that name as necessary.

dandavison force-pushed the type-assertions branch from 6e7bdd3 to f6d4657 Compare July 14, 2025 17:42

dandavison requested a review from cretz July 14, 2025 17:49

cretz approved these changes Jul 14, 2025

View reviewed changes

dandavison merged commit c8d54f8 into main Jul 14, 2025
10 checks passed

dandavison deleted the type-assertions branch July 14, 2025 19:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[2/5] Type-check assertions #14

[2/5] Type-check assertions #14

Uh oh!

dandavison commented Jul 12, 2025 •

edited

Loading

Uh oh!

dandavison Jul 14, 2025

Uh oh!

cretz left a comment

Uh oh!

dandavison commented Jul 14, 2025 •

edited

Loading

Uh oh!

cretz commented Jul 14, 2025

Uh oh!

cretz Jul 14, 2025

Uh oh!

dandavison Jul 14, 2025

Uh oh!

cretz Jul 14, 2025

Uh oh!

dandavison Jul 14, 2025

Uh oh!

cretz Jul 14, 2025

Uh oh!

dandavison Jul 14, 2025

Uh oh!

dandavison Jul 14, 2025

Uh oh!

Uh oh!

Uh oh!

		# This test is disabled since we currently have no way to be able to
		# assert-type-error-mypy on a line with a `type: ignore`.

		metafunc.parametrize("test_file", files_with_assertions, ids=lambda f: f.name)


		@pytest.mark.skipif(platform.system() == "Windows", reason="TODO: broken on Windows")

		import pytest


		def pytest_generate_tests(metafunc: pytest.Metafunc) -> None:

[2/5] Type-check assertions #14

[2/5] Type-check assertions #14

Uh oh!

Conversation

dandavison commented Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cretz left a comment

Choose a reason for hiding this comment

Uh oh!

dandavison commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cretz commented Jul 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dandavison commented Jul 12, 2025 •

edited

Loading

dandavison commented Jul 14, 2025 •

edited

Loading