bzlmod: fix _noop arity for check_direct_dependencies = "off"#2305
bzlmod: fix _noop arity for check_direct_dependencies = "off"#2305jayconrod merged 2 commits intobazel-contrib:masterfrom
Conversation
outdated_direct_dep_printer defaults to print (variadic). Both call sites (go_deps.bzl:636, :660) pass multiple positional arguments. When check_direct_dependencies = "off" swaps in _noop, the single-argument signature fails with: Error: _noop() accepts no more than 1 positional argument but got 5 tests/bcr/go_work already has a circl bazel_dep (1.3.8) vs pkg/go.mod (1.6.1) version skew that fires the :636 path; setting check_direct_dependencies = "off" there makes bazel build //... fail without this fix. Fixes bazel-contrib#2304.
There was a problem hiding this comment.
LGTM, thanks for fixing.
This is a small enough fix that I think a regression test isn't necessary. tests/bcr/go_work does cover it, but it runs as a separate CI job, not as part of bazel test //.... Ideally, we would test with something like //internal:bazel_test, but that test is very heavy: it creates a separate Bazel workspace where it downloads Go (EDIT: I think it actually tries to share downloads with the original repo, but I forget the details), builds the toolchain, and builds Gazelle from source (multiple times actually). So I'd hesitate to add another test like that.
cc @fmeum is there another way to test changes to go_deps?
|
@jayconrod @fmeum do you want me to drop the test change for this before merging? |
|
Let's keep the test change. Lacking a better place to put a regression test for now, some coverage is better than none. |
|
I don't see a better way either. I started building a unit testing framework for module extensions at some point, but that turned out to be surprisingly tricky. I might revisit this at some point though. |
outdated_direct_dep_printer defaults to print (variadic). Both call sites (go_deps.bzl:636, :660) pass multiple positional arguments. When check_direct_dependencies = "off" swaps in _noop, the single-argument signature fails with:
Error: _noop() accepts no more than 1 positional argument but got 5
tests/bcr/go_work already has a circl bazel_dep (1.3.8) vs pkg/go.mod (1.6.1) version skew that fires the :636 path; setting check_direct_dependencies = "off" there makes bazel build //... fail without this fix.
Fixes #2304