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

dev: address a few paper cuts #76415

Merged
merged 9 commits into from
Feb 12, 2022
Merged

Conversation

irfansharif
Copy link
Contributor

See individual commits, works through a few of the known paper cuts with dev/bazel (#75453).

@irfansharif irfansharif requested a review from a team as a code owner February 11, 2022 01:19
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@irfansharif irfansharif force-pushed the 220210.dev-misc branch 2 times, most recently from 3b6a77c to 5989638 Compare February 11, 2022 18:36
Copy link
Member

@RaduBerinde RaduBerinde left a comment

Choose a reason for hiding this comment

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

:lgtm_strong: Thanks for addressing all these, very cool!

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @irfansharif and @rickystewart)


-- commits, line 20 at r3:
[supernit] we were


go.mod, line 166 at r8 (raw file):

)

require github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510

FWIW, we use github.com/alessio/shellescape elsewhere

We set up a persistent bazel-remote cache process now through `dev
cache`, we no longer need this flag. We're also suggesting placing
these flags in appropriate bazelrcs instead.

Release note: None
This is what the Makefile does, and what folks expect.

Release note: None
Slipped in as part of cockroachdb#76189. Before this patch we were unintentionally
running all unit tests in whatever package was targeted. To repro, try:

    dev bench pkg/server -f='BenchmarkSetupSpanForIncomingRPC'

Release note: None
For sharded test packages, it doesn't make much sense to spawn multiple
test processes that don't end up running anything. Default to running
things in a single process if a filter is specified. To compare, try the
following before/after this commit:

  dev test pkg/server -f=TestClusterVersionUpgrade -v

Release note: None
It's a fairly common flag to use.

Release note: None
Unify a few codepaths while here.

Release note: None
It's closer to what folks would expect with per-user overrides.

Release note: None
It's useful to be able to pass arguments down to the go test binary
directly. This commit introduces a top-level flag to do just that. Now
we can do the following sort of thing:

    dev bench pkg/bench -f='BenchmarkTracing/1node/scan/trace=off' \
      --test-args '-test.memprofile=mem.out -test.cpuprofile=cpu.out'

Release note: None
Pick up changes in preceding commits.

Release note: None
Copy link
Contributor Author

@irfansharif irfansharif left a comment

Choose a reason for hiding this comment

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

TFTR!

bors r+

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained (waiting on @RaduBerinde and @rickystewart)


go.mod, line 166 at r8 (raw file):

Previously, RaduBerinde wrote…

FWIW, we use github.com/alessio/shellescape elsewhere

I was using it for the underlying shell lexer to step through tokens (there are two uses, both for datadriven and for --test-args); I don't think I can use shellescape for unless I'm missing something.

@irfansharif
Copy link
Contributor Author

bors r+

@craig
Copy link
Contributor

craig bot commented Feb 12, 2022

Already running a review

@craig
Copy link
Contributor

craig bot commented Feb 12, 2022

Build succeeded:

@craig craig bot merged commit 0aacc53 into cockroachdb:master Feb 12, 2022
@irfansharif irfansharif deleted the 220210.dev-misc branch February 12, 2022 01:49
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