Skip to content

Disable typing in py/private/pytest.py.tmpl #577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

Synss
Copy link
Contributor

@Synss Synss commented Jun 4, 2025

Disable type checking in generated code.

This fixes a regression that occurred after upgrading to the latest version of the rules. There, our tooling started producing the following typing errors.

bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:35: error: Cannot find implementation or library stub for module named "coverage"  [import-not-found]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:35: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:37: error: Argument 1 to "open" has incompatible type "str | None"; expected "int | str | bytes | PathLike[str] | PathLike[bytes]"  [arg-type]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:78: error: Argument 1 to "int" has incompatible type "str | None"; expected "str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc"  [arg-type]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:84: error: Argument 1 to "Path" has incompatible type "str | None"; expected "str | PathLike[str]"  [arg-type]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:109: error: Unsupported left operand type for + ("None")  [operator]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:109: note: Left operand is of type "str | None"
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:114: error: Argument 1 to "open" has incompatible type "str | None"; expected "int | str | bytes | PathLike[str] | PathLike[bytes]"  [arg-type]
Found 6 errors in 1 file (checked 1 source file)

Related:


Changes are visible to end-users: yes

This patch fixes a regression.

Ignore typing in generated code.

This fixes a regression that occurred after upgrading to the latest
version of the rules.  There, our tooling started producing the
following typing errors.

```
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:35: error: Cannot find implementation or library stub for module named "coverage"  [import-not-found]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:35: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:37: error: Argument 1 to "open" has incompatible type "str | None"; expected "int | str | bytes | PathLike[str] | PathLike[bytes]"  [arg-type]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:78: error: Argument 1 to "int" has incompatible type "str | None"; expected "str | Buffer | SupportsInt | SupportsIndex | SupportsTrunc"  [arg-type]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:84: error: Argument 1 to "Path" has incompatible type "str | None"; expected "str | PathLike[str]"  [arg-type]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:109: error: Unsupported left operand type for + ("None")  [operator]
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:109: note: Left operand is of type "str | None"
bazel-out/k8-fastbuild/bin/packages/cmk-agent-based/__test__.py:114: error: Argument 1 to "open" has incompatible type "str | None"; expected "int | str | bytes | PathLike[str] | PathLike[bytes]"  [arg-type]
Found 6 errors in 1 file (checked 1 source file)
```

Related:
 * aspect-build#428
@CLAassistant
Copy link

CLAassistant commented Jun 4, 2025

CLA assistant check
All committers have signed the CLA.

Copy link

aspect-workflows bot commented Jun 4, 2025

Test

5 test targets passed

Targets
//examples/multi_version:py_version_default_test [k8-fastbuild]                 2s
//examples/multi_version:py_version_test [k8-fastbuild-ST-494921797612]         2s
//examples/pytest:pytest_test [k8-fastbuild]                                    2s
//examples/pytest:sharded/test [k8-fastbuild]                                   4s
//examples/virtual_deps:pytest_test [k8-fastbuild]                              2s

Total test execution time was 11s. 33 tests (86.8%) were fully cached saving 54s.

@arrdem
Copy link
Collaborator

arrdem commented Jun 4, 2025

Thanks for the contribution, my take is that #570 is a better approach since it allows us to provide more verification of the base code we're delivering and opens the door to removing the need for a template action in the normal cases.

@Synss
Copy link
Contributor Author

Synss commented Jun 5, 2025

Yes, indeed. In the long-term, it's certainly be better to type and lint the file instead of silencing the type checker.

I'd nevertheless propose the following, pragmatic approach:

@arrdem
Copy link
Collaborator

arrdem commented Jun 6, 2025

Not my favorite but I agree it's incrementally less broken. Let's take it for now.

Copy link
Collaborator

@arrdem arrdem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@arrdem arrdem merged commit aae089f into aspect-build:main Jun 6, 2025
16 checks passed
@arrdem arrdem added this to the 1.6.0 milestone Jun 9, 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.

3 participants