Skip to content

fix(py_test): Support providing py_test's main from a separate package #603

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

plobsing
Copy link

@plobsing plobsing commented Jun 20, 2025

A call to py_test using a main from a separate package, such as

py_test(
    name = "repro",
    srcs = [
        "//common:test_main.py",  # Exported by //common:BUILD.bazel.
        "specific_test.py",       # A second srcs file is needed to force the failure.
    ],
    main = "//common:test_main.py",
)

works when using py_test from rules_python v1.4.1, but fails when using aspect_rules_py v1.6.0:

$ bazel test test:repro
INFO: Invocation ID: e1a579a3-5f06-48a4-93ac-493157878ad1
ERROR: /Users/peter/repro/test/BUILD.bazel:4:8: in determine_main rule //test:repro.find_main:
Traceback (most recent call last):
        File "/private/var/tmp/_bazel_peter/76f5e50945d4cebf81e863f9c06c01bc/external/aspect_rules_py+/py/private/py_executable.bzl", line 73, column 55, in _determine_main_impl
                return DefaultInfo(files = depset([_determine_main(ctx)]))
        File "/private/var/tmp/_bazel_peter/76f5e50945d4cebf81e863f9c06c01bc/external/aspect_rules_py+/py/private/py_executable.bzl", line 46, column 17, in _determine_main
                fail("could not find '{}' as specified by 'main' attribute".format(proposed_main))
Error in fail: could not find '//common:test_main.py' as specified by 'main' attribute
ERROR: /Users/peter/repro/test/BUILD.bazel:4:8: Analysis of target '//test:repro.find_main' failed
ERROR: Analysis of target '//test:repro' failed; build aborted: Analysis failed
INFO: Elapsed time: 0.140s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully
ERROR: No test targets were found, yet testing was requested

FWIW, I'm not sure this usage pattern is desirable; maybe I'll be able to do away with it in my codebase in the future. But it is an incompatibility that makes aspect_rules_py not a drop-in replacement for rules_python; I hit it while attempting to migrate.


Changes are visible to end-users: yes

  • Searched for relevant documentation and updated as needed: yes
  • Breaking change (forces users to change their own code or config): no
  • Suggested release notes appear below: yes

fix(py_test): Support providing py_test's main from a separate package

Test plan

  • Manual testing; please provide instructions so we can reproduce:

Create a small repro repo matching:

$ tree
.
├── common
│   ├── BUILD.bazel
│   └── test_main.py
├── MODULE.bazel
└── test
    ├── BUILD.bazel
    └── specific_test.py
$  cat MODULE.bazel
bazel_dep(name = "rules_python", version = "1.4.1")
bazel_dep(name = "aspect_rules_py", version = "1.6.0")

python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(python_version = "3.12", is_default = True)
$  cat common/BUILD.bazel
exports_files(["test_main.py"])
$  cat test/BUILD.bazel
# load("@rules_python//python:defs.bzl", "py_test")
load("@aspect_rules_py//py:defs.bzl", "py_test")

py_test(
    name = "repro",
    srcs = [
        "//common:test_main.py",
        "specific_test.py",
    ],
    main = "//common:test_main.py",
)
$  file common/test_main.py test/specific_test.py
common/test_main.py:   empty
test/specific_test.py: empty

And then bazel test test:repro.

A call to `py_test` using a `main` from a separate package, such as

```
py_test(
    name = "repro",
    srcs = [
        "//common:test_main.py",  # Exported by //common:BUILD.bazel.
        "specific_test.py",       # A second srcs file is needed to force the failure.
    ],
    main = "//common:test_main.py",
)
```

works when using `py_test` from `rules_python` v1.4.1, but fails when
using `aspect_rules_py` v1.6.0:

```
$ bazel test test:repro
INFO: Invocation ID: e1a579a3-5f06-48a4-93ac-493157878ad1
ERROR: /Users/peter/repro/test/BUILD.bazel:4:8: in determine_main rule //test:repro.find_main:
Traceback (most recent call last):
        File "/private/var/tmp/_bazel_peter/76f5e50945d4cebf81e863f9c06c01bc/external/aspect_rules_py+/py/private/py_executable.bzl", line 73, column 55, in _determine_main_impl
                return DefaultInfo(files = depset([_determine_main(ctx)]))
        File "/private/var/tmp/_bazel_peter/76f5e50945d4cebf81e863f9c06c01bc/external/aspect_rules_py+/py/private/py_executable.bzl", line 46, column 17, in _determine_main
                fail("could not find '{}' as specified by 'main' attribute".format(proposed_main))
Error in fail: could not find '//common:test_main.py' as specified by 'main' attribute
ERROR: /Users/peter/repro/test/BUILD.bazel:4:8: Analysis of target '//test:repro.find_main' failed
ERROR: Analysis of target '//test:repro' failed; build aborted: Analysis failed
INFO: Elapsed time: 0.140s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully
ERROR: No test targets were found, yet testing was requested
```

FWIW, I'm not sure this usage pattern is desirable; maybe I'll be able
to do away with it in my codebase in the future. But it is an
incompatibility that makes `aspect_rules_py` not a drop-in replacement
for `rules_python`; I hit it while attempting to migrate.
Copy link

aspect-workflows bot commented Jun 20, 2025

Test

All tests were cache hits

41 tests (100.0%) were fully cached saving 1m 9s.

@plobsing plobsing changed the title Support providing py_test's main from a separate package fix(py_test): Support providing py_test's main from a separate package Jun 20, 2025
@arrdem arrdem self-requested a review June 20, 2025 23:18
@arrdem arrdem added the bug Something isn't working label Jun 20, 2025
@arrdem
Copy link
Collaborator

arrdem commented Jun 20, 2025

Dupe of #582 I think.

@plobsing
Copy link
Author

Dupe of #582 I think.

Yes. Sorry I didn't scan the existing PRs; who would have figured that a longstanding defect affecting only a niche piece of functionality would hit two people in as many weeks.

IMO #582 is the better fix — it makes things even more consistent with how rules_python works, which should be the goal. However, it does change the type of an attribute on rules that are exposed in this ruleset's public API. Depending on how strict you want to be about back-compat, it may not be possible to do in a patch or minor version (I can come up with examples that would be broken, but I wouldn't call those uses "reasonable"). This change, is less ambitious by contrast, but I believe maintains greater back compatibility. Up to you how you want to proceed; feel free to close this PR out if you're going to get the other one in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants