From fd43709ae421303eb318731fac82536889616178 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Wed, 1 Feb 2023 15:12:31 +0000 Subject: [PATCH 1/5] Improve pyright verification of test cases in CI. Co-authored-by: Avasam --- .github/workflows/tests.yml | 4 ++-- pyrightconfig.json | 4 ++++ pyrightconfig.stricter.json | 2 ++ pyrightconfig.testcases.json | 1 + stubs/invoke/@tests/test_cases/check_task.py | 1 - stubs/protobuf/@tests/test_cases/check_struct.py | 1 - stubs/regex/@tests/test_cases/check_finditer.py | 2 -- stubs/requests/@tests/test_cases/check_post.py | 2 -- tests/check_consistent.py | 16 +++++++--------- 9 files changed, 16 insertions(+), 17 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 19f8d571f023..6b83c400645b 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -146,20 +146,20 @@ jobs: python-platform: ${{ matrix.python-platform }} python-version: ${{ matrix.python-version }} no-comments: ${{ matrix.python-version != '3.10' || matrix.python-platform != 'Linux' }} # Having each job create the same comment is too noisy. - project: ./pyrightconfig.stricter.json - uses: jakebailey/pyright-action@v1 with: version: ${{ steps.pyright_version.outputs.value }} python-platform: ${{ matrix.python-platform }} python-version: ${{ matrix.python-version }} no-comments: ${{ matrix.python-version != '3.10' || matrix.python-platform != 'Linux' }} # Having each job create the same comment is too noisy. - project: ./pyrightconfig.testcases.json + project: ./pyrightconfig.stricter.json - uses: jakebailey/pyright-action@v1 with: version: ${{ steps.pyright_version.outputs.value }} python-platform: ${{ matrix.python-platform }} python-version: ${{ matrix.python-version }} no-comments: ${{ matrix.python-version != '3.10' || matrix.python-platform != 'Linux' }} # Having each job create the same comment is too noisy. + project: ./pyrightconfig.testcases.json stub-uploader: name: Run the stub_uploader tests diff --git a/pyrightconfig.json b/pyrightconfig.json index 49419acd78e5..9fc8f5f5faa4 100644 --- a/pyrightconfig.json +++ b/pyrightconfig.json @@ -5,6 +5,10 @@ "stdlib", "stubs", ], + "exclude": [ + // test cases use a custom config file + "stubs/**/@tests/test_cases" + ], "typeCheckingMode": "basic", "strictListInference": true, "strictDictionaryInference": true, diff --git a/pyrightconfig.stricter.json b/pyrightconfig.stricter.json index e1f103e1c3b0..63c231e9dfe2 100644 --- a/pyrightconfig.stricter.json +++ b/pyrightconfig.stricter.json @@ -6,6 +6,8 @@ "stubs", ], "exclude": [ + // test cases use a custom pyrightconfig file + "stubs/**/@tests/test_cases", "stdlib/distutils/command", "stdlib/lib2to3/refactor.pyi", "stdlib/_tkinter.pyi", diff --git a/pyrightconfig.testcases.json b/pyrightconfig.testcases.json index 25c51569dd3b..13a7d8b37579 100644 --- a/pyrightconfig.testcases.json +++ b/pyrightconfig.testcases.json @@ -3,6 +3,7 @@ "typeshedPath": ".", "include": [ "test_cases", + "stubs/**/@tests/test_cases" ], "typeCheckingMode": "strict", // Using unspecific "type ignore" comments in test_cases. diff --git a/stubs/invoke/@tests/test_cases/check_task.py b/stubs/invoke/@tests/test_cases/check_task.py index 06b96ad7966f..a56d1488cb07 100644 --- a/stubs/invoke/@tests/test_cases/check_task.py +++ b/stubs/invoke/@tests/test_cases/check_task.py @@ -1,4 +1,3 @@ -# pyright: reportUnnecessaryTypeIgnoreComment=true from __future__ import annotations from invoke import Context, task diff --git a/stubs/protobuf/@tests/test_cases/check_struct.py b/stubs/protobuf/@tests/test_cases/check_struct.py index 87bb72264fb5..d3679af470cd 100644 --- a/stubs/protobuf/@tests/test_cases/check_struct.py +++ b/stubs/protobuf/@tests/test_cases/check_struct.py @@ -1,4 +1,3 @@ -# pyright: reportUnnecessaryTypeIgnoreComment=true from __future__ import annotations from google.protobuf.struct_pb2 import ListValue, Struct diff --git a/stubs/regex/@tests/test_cases/check_finditer.py b/stubs/regex/@tests/test_cases/check_finditer.py index 518a4a5a3d42..0b572973ceaf 100644 --- a/stubs/regex/@tests/test_cases/check_finditer.py +++ b/stubs/regex/@tests/test_cases/check_finditer.py @@ -1,5 +1,3 @@ -# pyright: reportUnnecessaryTypeIgnoreComment=true - from __future__ import annotations from typing import List diff --git a/stubs/requests/@tests/test_cases/check_post.py b/stubs/requests/@tests/test_cases/check_post.py index 59c75395e961..d68a484d9c8e 100644 --- a/stubs/requests/@tests/test_cases/check_post.py +++ b/stubs/requests/@tests/test_cases/check_post.py @@ -1,5 +1,3 @@ -# pyright: reportUnnecessaryTypeIgnoreComment=true - from __future__ import annotations from collections.abc import Iterable diff --git a/tests/check_consistent.py b/tests/check_consistent.py index 73f1b8b7cf7a..d09559483643 100644 --- a/tests/check_consistent.py +++ b/tests/check_consistent.py @@ -65,9 +65,15 @@ def check_stubs() -> None: allowed = {"METADATA.toml", "README", "README.md", "README.rst", "@tests"} assert_consistent_filetypes(dist, kind=".pyi", allowed=allowed) + tests_dir = dist / "@tests" + if tests_dir.exists() and tests_dir.is_dir(): + py_files_present = bool([file for file in tests_dir.iterdir() if file.suffix == ".py"]) + error_message = "Test-case files must be in an `@tests/test_cases/` directory, not in the `@tests/` directory" + assert not py_files_present, error_message + def check_test_cases() -> None: - for package_name, testcase_dir in get_all_testcase_directories(): + for _, testcase_dir in get_all_testcase_directories(): assert_consistent_filetypes(testcase_dir, kind=".py", allowed={"README.md"}, allow_nonidentifier_filenames=True) bad_test_case_filename = 'Files in a `test_cases` directory must have names starting with "check_"; got "{}"' for file in testcase_dir.rglob("*.py"): @@ -75,14 +81,6 @@ def check_test_cases() -> None: with open(file, encoding="UTF-8") as f: lines = {line.strip() for line in f} assert "from __future__ import annotations" in lines, "Test-case files should use modern typing syntax where possible" - if package_name != "stdlib": - pyright_setting_not_enabled_msg = ( - f'Third-party test-case file "{file}" must have ' - f'"# pyright: reportUnnecessaryTypeIgnoreComment=true" ' - f"at the top of the file" - ) - has_pyright_setting_enabled = "# pyright: reportUnnecessaryTypeIgnoreComment=true" in lines - assert has_pyright_setting_enabled, pyright_setting_not_enabled_msg def check_no_symlinks() -> None: From 0891ef1633639a523b8659d5b275924e9040c3b0 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Wed, 1 Feb 2023 15:19:21 +0000 Subject: [PATCH 2/5] Add some unnecessary `type: ignore`s to some test cases to show that pyright now fails CI --- stubs/invoke/@tests/test_cases/check_task.py | 2 +- stubs/requests/@tests/test_cases/check_post.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stubs/invoke/@tests/test_cases/check_task.py b/stubs/invoke/@tests/test_cases/check_task.py index a56d1488cb07..4c14a2ed3469 100644 --- a/stubs/invoke/@tests/test_cases/check_task.py +++ b/stubs/invoke/@tests/test_cases/check_task.py @@ -13,5 +13,5 @@ def docker_build(context: Context) -> None: @task(docker_build) -def docker_push(context: Context) -> None: +def docker_push(context: Context) -> None: # type: ignore pass diff --git a/stubs/requests/@tests/test_cases/check_post.py b/stubs/requests/@tests/test_cases/check_post.py index d68a484d9c8e..1f2754710108 100644 --- a/stubs/requests/@tests/test_cases/check_post.py +++ b/stubs/requests/@tests/test_cases/check_post.py @@ -12,7 +12,7 @@ # ================================================================================================= -url = "https://httpbin.org/post" +url = "https://httpbin.org/post" # type: ignore multiple_files = [ ("images", ("foo.png", open("foo.png", "rb"), "image/png")), ("images", ("bar.png", open("bar.png", "rb"), "image/png")), From 83b28f6ddc58cba69c9e80af19bbf8999e8536c9 Mon Sep 17 00:00:00 2001 From: AlexWaygood Date: Wed, 1 Feb 2023 15:27:05 +0000 Subject: [PATCH 3/5] Revert "Add some unnecessary `type: ignore`s to some test cases to show that pyright now fails CI" This reverts commit 0891ef1633639a523b8659d5b275924e9040c3b0. --- stubs/invoke/@tests/test_cases/check_task.py | 2 +- stubs/requests/@tests/test_cases/check_post.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/stubs/invoke/@tests/test_cases/check_task.py b/stubs/invoke/@tests/test_cases/check_task.py index 4c14a2ed3469..a56d1488cb07 100644 --- a/stubs/invoke/@tests/test_cases/check_task.py +++ b/stubs/invoke/@tests/test_cases/check_task.py @@ -13,5 +13,5 @@ def docker_build(context: Context) -> None: @task(docker_build) -def docker_push(context: Context) -> None: # type: ignore +def docker_push(context: Context) -> None: pass diff --git a/stubs/requests/@tests/test_cases/check_post.py b/stubs/requests/@tests/test_cases/check_post.py index 1f2754710108..d68a484d9c8e 100644 --- a/stubs/requests/@tests/test_cases/check_post.py +++ b/stubs/requests/@tests/test_cases/check_post.py @@ -12,7 +12,7 @@ # ================================================================================================= -url = "https://httpbin.org/post" # type: ignore +url = "https://httpbin.org/post" multiple_files = [ ("images", ("foo.png", open("foo.png", "rb"), "image/png")), ("images", ("bar.png", open("bar.png", "rb"), "image/png")), From dfabe2d9be2777d26433d4ecdaad5d82485535dc Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Wed, 1 Feb 2023 15:56:22 +0000 Subject: [PATCH 4/5] Name the steps --- .github/workflows/tests.yml | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 6b83c400645b..9b5911cfdc53 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -140,20 +140,23 @@ jobs: with: file: "pyproject.toml" field: "tool.typeshed.pyright_version" - - uses: jakebailey/pyright-action@v1 + - name: Run pyright with basic settings on all the stubs + uses: jakebailey/pyright-action@v1 with: version: ${{ steps.pyright_version.outputs.value }} python-platform: ${{ matrix.python-platform }} python-version: ${{ matrix.python-version }} no-comments: ${{ matrix.python-version != '3.10' || matrix.python-platform != 'Linux' }} # Having each job create the same comment is too noisy. - - uses: jakebailey/pyright-action@v1 + - name: Run pyright with stricter settings on some of the stubs + uses: jakebailey/pyright-action@v1 with: version: ${{ steps.pyright_version.outputs.value }} python-platform: ${{ matrix.python-platform }} python-version: ${{ matrix.python-version }} no-comments: ${{ matrix.python-version != '3.10' || matrix.python-platform != 'Linux' }} # Having each job create the same comment is too noisy. project: ./pyrightconfig.stricter.json - - uses: jakebailey/pyright-action@v1 + - name: Run pyright on the test cases + uses: jakebailey/pyright-action@v1 with: version: ${{ steps.pyright_version.outputs.value }} python-platform: ${{ matrix.python-platform }} From 1cec6262bb169fa4121813a82f3fa21fcc6ed567 Mon Sep 17 00:00:00 2001 From: Alex Waygood Date: Tue, 7 Feb 2023 11:46:38 +0000 Subject: [PATCH 5/5] facepalm --- tests/check_consistent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/check_consistent.py b/tests/check_consistent.py index d09559483643..fd0111bb0c4d 100644 --- a/tests/check_consistent.py +++ b/tests/check_consistent.py @@ -67,7 +67,7 @@ def check_stubs() -> None: tests_dir = dist / "@tests" if tests_dir.exists() and tests_dir.is_dir(): - py_files_present = bool([file for file in tests_dir.iterdir() if file.suffix == ".py"]) + py_files_present = any(file.suffix == ".py" for file in tests_dir.iterdir()) error_message = "Test-case files must be in an `@tests/test_cases/` directory, not in the `@tests/` directory" assert not py_files_present, error_message