Skip to content
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

lint: make runnable under bazel test #119589

Merged
merged 1 commit into from
Mar 6, 2024

Conversation

rickystewart
Copy link
Collaborator

@rickystewart rickystewart commented Feb 23, 2024

This test was never directly runnable under bazel test, and instead
required using bazel run with the wrapper target
//build/bazelutil:lint that set up some dependencies and some
environment variables appropriately. Instead, we simply pass in those
environment variables from the CI script or from dev, and add extra
dependencies to the lint_test target (crlfmt, optfmt) so the test
can use them.

It's necessary to pass GO_SDK as a path to the Bazel-provided Go SDK
and COCKROACH_WORKSPACE as the path to the checkout as this stuff will
not be found in the sandbox.

Epic: CRDB-8308
Release note: None

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@rickystewart rickystewart force-pushed the lint-refactor branch 5 times, most recently from 996f852 to 7c0f4b6 Compare February 23, 2024 20:56
@rickystewart rickystewart changed the title [WIP] lint: make runnable under bazel test lint: make runnable under bazel test Feb 23, 2024
@rickystewart rickystewart force-pushed the lint-refactor branch 3 times, most recently from dc6ea05 to 0a0d4cd Compare February 23, 2024 21:21
@rickystewart rickystewart force-pushed the lint-refactor branch 2 times, most recently from 6bf2e1c to a395a4f Compare March 5, 2024 22:51
@rickystewart rickystewart requested a review from rail March 5, 2024 22:51
@rickystewart rickystewart marked this pull request as ready for review March 5, 2024 22:51
@rickystewart rickystewart requested a review from a team as a code owner March 5, 2024 22:51
This test was never directly runnable under `bazel test`, and instead
required using `bazel run` with the wrapper target
`//build/bazelutil:lint` that set up some dependencies and some
environment variables appropriately. Instead, we simply pass in those
environment variables from the CI script or from `dev`, and add extra
dependencies to the `lint_test` target (`crlfmt`, `optfmt`) so the test
can use them.

It's necessary to pass `GO_SDK` as a path to the Bazel-provided Go SDK
and `COCKROACH_WORKSPACE` as the path to the checkout as this stuff will
not be found in the sandbox.

Epic: CRDB-8308
Release note: None
@rickystewart
Copy link
Collaborator Author

bors r=rail

@craig
Copy link
Contributor

craig bot commented Mar 6, 2024

Build succeeded:

@craig craig bot merged commit c3c4486 into cockroachdb:master Mar 6, 2024
17 of 18 checks passed
@rickystewart rickystewart mentioned this pull request Mar 7, 2024
rickystewart added a commit to rickystewart/cockroach that referenced this pull request Mar 7, 2024
Regressed by cockroachdb#119589

Closes cockroachdb#120033

Epic: none
Release note: None
craig bot pushed a commit that referenced this pull request Mar 7, 2024
120071: lint_urls: fix script r=rail a=rickystewart

Regressed by #119589

Closes #120033

Epic: none
Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
herkolategan added a commit to herkolategan/cockroach that referenced this pull request Mar 20, 2024
A change was recently added to a package `init` that causes a failure when
listing the benchmarks for that package. These packages should be excluded from
any interaction, to avoid complete execution failure.

This is slightly different from the other `exclude` flag that is meant to not
execute specific benchmarks within a given package.

See: cockroachdb#119589

Epic: None
Release Note: None
herkolategan added a commit to herkolategan/cockroach that referenced this pull request Mar 21, 2024
Recently, `pkg/testutils/lint/lint_test.go` (cockroachdb#119589) added logic to `init`, that requires
a `GO_SDK` env var that causes a failure when listing the benchmarks for that
package, since on the target microbenchmark env there is no such env var
specified. It's possible to add the env var to avoid failure. But in the future
more logic that could probe the env var further could still cause a failure as
go will not be installed on these machines. These packages should be excluded
from any interaction, to avoid complete execution failure.

This is slightly different from the other `exclude` flag that is meant to not
execute specific benchmarks within a given package.

See: cockroachdb#119589

Epic: None
Release Note: None
craig bot pushed a commit that referenced this pull request Mar 22, 2024
120752: roachprod-microbench,build: ignore packages r=renatolabs,srosenberg a=herkolategan

Recently, `pkg/testutils/lint/lint_test.go` (#119589) added logic to `init`, that requires
a `GO_SDK` env var that causes a failure when listing the benchmarks for that
package, since on the target microbenchmark env there is no such env var
specified. It's possible to add the env var to avoid failure. But in the future
more logic that could probe the env var further could still cause a failure as
go will not be installed on these machines. These packages should be excluded
from any interaction, to avoid complete execution failure.

This is slightly different from the other `exclude` flag that is meant to not
execute specific benchmarks within a given package.

See: #119589

Epic: None
Release Note: None

Co-authored-by: Herko Lategan <[email protected]>
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