Merge #122048 #122727 #122903

122048: workflows: make `unit tests` build mandatory in GitHub Actions r=rail a=rickystewart

And delete related TC scripts. Note that the unit tests job in GitHub Actions subsumes the unit test jobs in TC as well as the UI lint and UI test job.

Epic: CRDB-8308
Release note: None

122727: workflows: add staging branches to the patterns r=rickystewart a=rail

Previously, the actions on the staging-* branches were ignored. These branches are used for extraordinary releases.

This PR adjusts the patterns to include the staging branches.

Epic: none
Release note: None

122903: workload/tpcc: match against correct ErrNoRows in delivery txn r=RaduBerinde a=nvanbenschoten

Fixes #122891.

`sql.ErrNoRows` is not the same as `pgx.ErrNoRows`. As a result, this logic has been broken since TPC-C was ported to use pgx in d15637c (2018). This commit fixes the broken logic by matching on the correct error.

Release note: None

Co-authored-by: Ricky Stewart <[email protected]>
Co-authored-by: Rail Aliiev <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>

.github/bors.toml

@@ -3,7 +3,6 @@
 # List of commit statuses that must pass on the merge commit before it is
 # pushed to master.
 status = [
-       "Bazel Essential CI (Cockroach)",
        "acceptance",
        "check_generated_code",
        "docker_image_amd64",
@@ -16,6 +15,7 @@ status = [
        "local_roachtest_fips",
        "macos_amd64_build",
        "macos_arm64_build",
+       "unit_tests",
        "windows_build",
 ]
 

.github/workflows/github-actions-essential-ci.yml

@@ -1,44 +1,52 @@
 name: GitHub Actions Essential CI
 on:
   pull_request:
-    types: [ opened, reopened, synchronize ]
+    types: [opened, reopened, synchronize]
     branches:
-      - 'master'
-      - 'release-*'
-      - '!release-1.0*'
-      - '!release-1.1*'
-      - '!release-2.0*'
-      - '!release-2.1*'
-      - '!release-19.1*'
-      - '!release-19.2*'
-      - '!release-20.1*'
-      - '!release-20.2*'
-      - '!release-21.1*'
-      - '!release-21.2*'
-      - '!release-22.1*'
-      - '!release-22.2*'
-      - '!release-23.1*'
-      - '!release-23.2*'
+      - "master"
+      - "release-*"
+      - "staging-*"
+      - "!release-1.0*"
+      - "!release-1.1*"
+      - "!release-2.0*"
+      - "!release-2.1*"
+      - "!release-19.1*"
+      - "!release-19.2*"
+      - "!release-20.1*"
+      - "!release-20.2*"
+      - "!release-21.1*"
+      - "!release-21.2*"
+      - "!release-22.1*"
+      - "!release-22.2*"
+      - "!release-23.1*"
+      - "!release-23.2*"
+      - "!staging-v22.2*"
+      - "!staging-v23.1*"
+      - "!staging-v23.2*"
   push:
     branches:
-      - 'master'
-      - 'release-*'
-      - 'staging'
-      - 'trying'
-      - '!release-1.0*'
-      - '!release-1.1*'
-      - '!release-2.0*'
-      - '!release-2.1*'
-      - '!release-19.1*'
-      - '!release-19.2*'
-      - '!release-20.1*'
-      - '!release-20.2*'
-      - '!release-21.1*'
-      - '!release-21.2*'
-      - '!release-22.1*'
-      - '!release-22.2*'
-      - '!release-23.1*'
-      - '!release-23.2*'
+      - "master"
+      - "release-*"
+      - "staging-*"
+      - "staging"
+      - "trying"
+      - "!release-1.0*"
+      - "!release-1.1*"
+      - "!release-2.0*"
+      - "!release-2.1*"
+      - "!release-19.1*"
+      - "!release-19.2*"
+      - "!release-20.1*"
+      - "!release-20.2*"
+      - "!release-21.1*"
+      - "!release-21.2*"
+      - "!release-22.1*"
+      - "!release-22.2*"
+      - "!release-23.1*"
+      - "!release-23.2*"
+      - "!staging-v22.2*"
+      - "!staging-v23.1*"
+      - "!staging-v23.2*"
 concurrency:
   group: ${{ github.head_ref || github.run_id }}
   cancel-in-progress: true
@@ -269,7 +277,7 @@ jobs:
       - name: clean up
         run: ./build/github/cleanup-engflow-keys.sh
         if: always()
-  EXPERIMENTAL_unit_tests:
+  unit_tests:
     runs-on: [self-hosted, basic_runner_group]
     steps:
       - uses: actions/checkout@v4
@@ -282,7 +290,7 @@ jobs:
       - run: ./build/github/get-engflow-keys.sh
       - run: ./build/github/prepare-summarize-build.sh
       - name: run tests
-        run: bazel test //pkg:all_tests //pkg/ui:lint //pkg/ui:test --config crosslinux --jobs 300 --remote_download_minimal --bes_keywords ci-unit-test --config=use_ci_timeouts --build_event_binary_file=bes.bin $(./build/github/engflow-args.sh)
+        run: ./build/github/unit-tests.sh
       - name: upload test results
         run: ./build/github/summarize-build.sh bes.bin
         if: always()

build/github/unit-tests.sh

@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+
+set -euxo pipefail
+
+EXTRA_PARAMS=""
+
+if [ "$GITHUB_ACTIONS_BRANCH" == "staging" ]; then
+  # enable up to 1 retry (2 attempts, worst-case) per test executable to report flakes but only on release branches and staging.
+  EXTRA_PARAMS=" --flaky_test_attempts=2"
+fi
+
+
+bazel test //pkg:all_tests //pkg/ui:lint //pkg/ui:test \
+    --config crosslinux --jobs 300 --remote_download_minimal \
+    --bes_keywords ci-unit-test --config=use_ci_timeouts \
+    --build_event_binary_file=bes.bin $(./build/github/engflow-args.sh) \
+    $EXTRA_PARAMS

build/teamcity/cockroach/ci/tests-aws-linux-arm64-bigvm/unit_tests.sh

@@ -2,4 +2,11 @@
 
 set -euo pipefail
 
-source build/teamcity/cockroach/ci/tests/unit_tests.sh
+dir="$(dirname $(dirname $(dirname $(dirname $(dirname "${0}")))))"
+
+source "$dir/teamcity-support.sh"  # For $root
+source "$dir/teamcity-bazel-support.sh"  # For run_bazel
+
+tc_start_block "Run unit tests"
+BAZEL_SUPPORT_EXTRA_DOCKER_ARGS="-e TC_BUILD_BRANCH -e GITHUB_API_TOKEN -e BUILD_VCS_NUMBER -e TC_BUILD_ID -e TC_SERVER_URL -e TC_BUILDTYPE_ID -e GITHUB_REPO" run_bazel build/teamcity/cockroach/ci/tests-aws-linux-arm64-bigvm/unit_tests_impl.sh
+tc_end_block "Run unit tests"

build/teamcity/cockroach/ci/tests/unit_tests_ccl_impl.sh → build/teamcity/cockroach/ci/tests-aws-linux-arm64-bigvm/unit_tests_impl.sh

@@ -16,5 +16,5 @@ if tc_bors_branch; then
 fi
 
 $(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci -- test --config=ci --config=use_ci_timeouts -c fastbuild \
-                                  //pkg:ccl_tests \
-                                   --profile=/artifacts/profile.gz $EXTRA_PARAMS
+    //pkg:all_tests \
+    --profile=/artifacts/profile.gz $EXTRA_PARAMS

pkg/workload/tpcc/delivery.go

@@ -12,7 +12,6 @@ package tpcc
 
 import (
 	"context"
-	gosql "database/sql"
 	"fmt"
 	"sort"
 	"strings"
@@ -96,10 +95,10 @@ func (del *delivery) run(
 				var oID int
 				if err := del.selectNewOrder.QueryRowTx(ctx, tx, wID, dID).Scan(&oID); err != nil {
 					// If no matching order is found, the delivery of this order is skipped.
-					if !errors.Is(err, gosql.ErrNoRows) {
-						del.config.auditor.skippedDelivieries.Add(1)
+					if !errors.Is(err, pgx.ErrNoRows) {
 						return errors.Wrap(err, "select new_order failed")
 					}
+					del.config.auditor.skippedDelivieries.Add(1)
 					continue
 				}
 				dIDoIDPairs[dID] = oID