x/telemetry/godev/cmd/telemetrygodev: test fails when run in a standalone module #65258
Comments
bcmills
added
Testing
|
Reopened #34352 for the missing LUCI builder coverage. |
|
The test failure was introduced in https://go.dev/cl/556860 (attn @findleyr @hyangah). |
|
Change https://go.dev/cl/557839 mentions this issue: |
|
The module is not intended to run as a standalone module. It is tightly coupled with the x/telemetry module and the intention is to test the three modules in the repo are consistent. There is also no reason to run any tests in this module on android. I think the right fix is to disable the tests in unintended platform completely. |
|
If it is tightly coupled with (What is the value of keeping the nested |
The go toolchains will depend on
|
|
Module graph pruning should prevent the dependencies of We already rely on that, for example, to prune out the dependency on |
|
Is there a reason that we need to run telemetry.go.dev on android? This is a result of a long discussion to keep the x/telemetry module minimal and small. (no typescript or media files in the module people need to import). |
|
FWIW, I don't think it matters in this test (see the trivial fix), but agree that we don't need to test the godev sub module on anything but linux. However, this is probably unnecessarily complicated to configure on the builders: we definitely want to run x/telemetry module tests on all platforms. We can skip non-target platforms in our tests of the godev module as necessary. But it's not necessary in this case. |
|
I am more concerned about the impact on fixing #34352 than about running on Android in particular. An exemption to allow For comparison, consider |
|
As our project portfolio grows, I think we need to reevaluate the requirements of #34352. Not all Go modules our team is working on are intended to be hosting packages and tools that can be gettable or installed on by external users. For this specific issue, I think we can just fix the tests.
|
|
Change https://go.dev/cl/563358 mentions this issue: |
golang.org/x/telemetry/godev is a module that hosts code for the telemetry.go.dev service. This module is not intended to work standalone, but assumes the whole telemetry repo is cloned. Running this service on platforms like android, ios, etc is not our focus either. The production service is likely running on linux, but we also want to be able to test, develop on darwin or windows. * Skip platforms that are not likely used by x/telemetry/godev contributors. e.g. android, ios, ... * Skip if 'go' is not available. * Run go list to find x/telemetry module. When go list runs from a directory under x/telemetry/godev, the command will return the replaced directory path, which is also the repo root. For golang/go#65549 For golang/go#65258 Change-Id: I88f5f51bd0f2c355975127e1fa9559394e307f97 Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/563358 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com> Auto-Submit: Hyang-Ah Hana Kim <hyangah@gmail.com> Run-TryBot: Hyang-Ah Hana Kim <hyangah@gmail.com>
Go version
6e7aee5
Output of
go envin your module/workspace:What did you do?
https://build.golang.org/log/a1fcb0cfb4ce3595205204cea723c2d7a5c447e5
What did you see happen?
Note that the
androidbuilder runs tests by copying the entire module containing the package to the builder, similar to runninggo teston the package from outside of its module.The
replacedirective added in https://go.dev/cl/499918 causes thegolang.org/x/telemetry/godevmodule to depend on an invalid version ofgolang.org/x/telemetry, but if you alsogo geta valid version of that module, the failure is also reproducible on any platform by running the test from the module cache:Per #34352 (comment) the test failure should also be reproducible on the LUCI builders, but for some reason it is not — no failures are showing up on https://ci.chromium.org/p/golang/g/x-telemetry-gotip/console. I will follow up on that separately.
What did you expect to see?
All tests passing, including when run from within the module cache.
The text was updated successfully, but these errors were encountered: