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: warn or error if you try to dev test a target that cannot be bazel test'ed #107813

Closed
rickystewart opened this issue Jul 28, 2023 · 0 comments · Fixed by #109288
Closed

dev: warn or error if you try to dev test a target that cannot be bazel test'ed #107813

rickystewart opened this issue Jul 28, 2023 · 0 comments · Fixed by #109288
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-dev-inf

Comments

@rickystewart
Copy link
Collaborator

rickystewart commented Jul 28, 2023

Some things in tree that look like "tests" are not really proper Bazel tests because they cannot be tested in the sandbox. dev can detect these and suggest running them in a different way.

cockroach$ bazel query "attr(tags, \"integration\", kind(go_test, pkg/...))"
//pkg/acceptance:acceptance_test
//pkg/compose:compose_test
//pkg/compose/compare/compare:compare_test
//pkg/testutils/docker:docker_test
//pkg/testutils/lint:lint_test

Epic: CRDB-17171

Jira issue: CRDB-30203

@rickystewart rickystewart added C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) A-build-system T-dev-inf labels Jul 28, 2023
liamgillies pushed a commit that referenced this issue Aug 23, 2023
Running `dev test` on these integration tests will
always fail, so this PR adds a warning when running
the command on those files.

Fixes: #107813
Release note: None
craig bot pushed a commit that referenced this issue Aug 23, 2023
…109356

108485: github: code coverage workflows r=RaduBerinde a=RaduBerinde

This change adds two GitHub Action workflows which run on each PR. One generates unit test code coverage data, and one publishes that data to a GCS bucket from where Reviewable can access it.

We generate coverage data using `bazel coverage`, but we restrict it to only test the packages that have been modified by the PR.

Two workflows are required for security (the first workflow runs potentially malicious code from a fork); for more details, see https://securitylab.github.com/research/github-actions-preventing-pwn-requests/

Epic: none
Release note: None

109036: roachprod: better determination if scp -R flag can be used r=RaduBerinde a=RaduBerinde

When uploading a file to a cluster, we use the "tree dist" algorithm by default. This uploads the file to a single node, then we copy the file from that node to the other nodes (up to 10).

This only makes sense if the remote-to-remote transfers can happen directly, which only happens if we pass the `-R -A` flags to `scp`. Unfortunately older versions don't support these flags. Currently the flags are only passed if the OS is `darwin`.

This commits improves the determination - we run `ssh -V` (once) and check if the `SSL` major version is three. For reference, some examples of what `ssh -V` returns:
 - recent MacOSX: `OpenSSH_9.0p1, LibreSSL 3.3.6`
 - Ubuntu 22.04: `OpenSSH_8.9p1 Ubuntu-3ubuntu0.3, OpenSSL 3.0.2 15 Mar 2022`

In addition, if the version is not 3, we disable the use of "tree dist".

Epic: none
Release note: None

109260: sql stats: skip tests hitting combinedstmts and statements endpoints r=gtr a=gtr

Part of #109184.

This commit skips tests which hit the `combinedStmts` or `statements` endpoints which will sometimes timeout under stress as a result of recent backend changes. The test investigation is tracked by #109184.

Release note: None

109288: dev: error when trying to `dev test` a bazel tested target r=rickystewart a=liamgillies

Running `dev test` on these integration tests will always fail, so this PR adds a error when running the command on those files.

Fixes: #107813
Release note: None

109292: sql: fix expected batch count for edge case in copy test r=rharding6373 a=rharding6373

In TestLargeDynamicRows we test that 4 rows of data can fit in a batch size of at least 4 rows given default memory sizes. However, when we set the batch row size to the minimum value of 4, the test hook that counts batches counts an extra empty batch. This PR changes adjusts the minimum row size to 5 for the purposes of this test.

Epic: None
Fixes: #109134

Release note: None

109324: build: update bazel builder build docs r=rickystewart a=rail

Previously, the documentation described a manual build of the `bazelbuilder` docker image. The current approach is to use CI to build the image.

This PR updates the documentation to reflect the current process, including the FIPS image steps.

Epic: none
Release note: None

109340: changefeedccl: move node drain handling logic out of kvfeed r=miretskiy a=jayshrivastava

Previously, the kvfeed was responsible for monitoring for
node drains using a goroutine. This change moves this logic
into the change aggregator and removes the goroutine.
Overall, this change makes the code more organized and performant.

This change was inspired by work being done for #109167. The
work in that PR requires being able to restart the kvfeed.
Having drain logic intermingled with the kvfeed makes
restarts much more complex, hard to review, prone to bugs, etc.

Informs: #96953
Release note: None
Epic: None

109349: kv: wait on latches on each key in reverse acquisition order r=arulajmani,kvoli a=nvanbenschoten

This commit allocates latch IDs from the top of the uint64 space and in reverse order. This is done to order latches in the tree on a same key in reverse order of acquisition. Doing so ensures that when we iterate over the tree and see a key with many conflicting latches, we visit the latches on that key in the reverse order that they will be released. In doing so, we minimize the number of open channels that we wait on (calls to `waitForSignal`) and minimize the number of goroutine scheduling points. This is important to avoid spikes in runnable goroutine after each request completes, which can negatively affect node health.

See experiments below.

Epic: None
Release note (performance improvement): The impact of high concurrency blind writes to the same key on goroutine scheduling latency was reduced.

109356: build: explicitly set SKIP_LABEL_TEST_FAILURE in compose.sh r=rickystewart a=chrisseto

Previously, `SKIP_LABEL_TEST_FAILURE` was being set via a teamcity configuration. This change was quite opaque as the majority of CI configuration for Cockroach is stored as shell scripts within its repo. This commit follows that pattern by explicitly setting `SKIP_LABEL_TEST_FAILURE` in the script that runs `TestComposeCompare`.

Epic: None
Release note: None

Co-authored-by: Radu Berinde <[email protected]>
Co-authored-by: gtr <[email protected]>
Co-authored-by: Liam Gillies <[email protected]>
Co-authored-by: rharding6373 <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Jayant Shrivastava <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
Co-authored-by: Chris Seto <[email protected]>
@craig craig bot closed this as completed in e6e8b64 Aug 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-build-system C-enhancement Solution expected to add code/behavior + preserve backward-compat (pg compat issues are exception) T-dev-inf
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant