Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions .github/workflows/nightly.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@ jobs:
uses: ./.github/workflows/test_workflow.yml
with:
configuration: '["asan"]'
# C++ gtests (ASan + TSan) run on every PR via native-sanitizer-tests in ci.yml.
# Skip them here so the nightly focuses on Java functional tests under ASan.
# C++ gtests run on every branch push from .gitlab/sanitizer-tests, under
# ASan, TSan and release. Skip them here so the nightly focuses on Java
# functional tests under ASan.
skip_gtest: true
run-slow-test:
uses: ./.github/workflows/test_workflow.yml
Expand All @@ -28,8 +29,9 @@ jobs:
uses: ./.github/workflows/test_workflow.yml
with:
configuration: '["asan"]'
# C++ gtests (ASan + TSan) run on every PR via native-sanitizer-tests in ci.yml.
# Skip them here so the nightly focuses on Java functional tests under ASan.
# C++ gtests run on every branch push from .gitlab/sanitizer-tests, under
# ASan, TSan and release. Skip them here so the nightly focuses on Java
# functional tests under ASan.
skip_gtest: true
slow_tests: true
fuzz:
Expand Down
9 changes: 9 additions & 0 deletions .gitlab/build-deploy/.gitlab-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,15 @@ build-artifact:
- job: gtest-tsan-arm64
artifacts: false
optional: true
# Not optional, for the same reason as the ASan jobs: these share
# .sanitizer_job's rules exactly, so whenever this job exists they exist
# too. Leaving them out of the DAG would let an -O3-only failure publish
# an artifact before the pipeline went red, which is the failure this
# gate is for.
- job: gtest-release-amd64
artifacts: false
- job: gtest-release-arm64
artifacts: false
rules:
- !reference [.skip-on-release, rules]
- when: on_success
Expand Down
28 changes: 27 additions & 1 deletion .gitlab/sanitizer-tests/.gitlab-ci.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
# C++ unit tests under ASan and TSan.
# C++ unit tests under ASan, TSan, and release optimisation.
#
# These run on every branch push (not MR pipelines — GitHub Actions handles those).
#
# The release configuration is here rather than left to the label-gated Java
# matrix because it is the only one that compiles the gtests with optimisation.
# Both sanitizer presets pass -fno-optimize-sibling-calls and no -O, so any
# defect that only appears once the optimiser runs -- a fixture frame folded
# away by a tail call, say -- is invisible to every other job in the pipeline.
#
# Strategy: use Gradle only for compile+link (buildGtest{Config}), then run
# each binary directly from the shell. This bypasses Gradle's daemon I/O
# which swallows child process output when fd 1/2 are not the terminal.
Expand Down Expand Up @@ -86,6 +92,26 @@ gtest-asan-arm64:
SANITIZER_CONFIG: Asan
SANITIZER_LC: asan

# Not a sanitizer, but it reuses .sanitizer_job unchanged: buildGtestRelease
# exists alongside the sanitizer variants and emits binaries under the same
# bin/gtest/<config>_<test>/ layout the runner loop already globs for. The
# SANITIZER_* variable names are a misnomer here and nothing more.
gtest-release-amd64:
extends: .sanitizer_job
Comment on lines +99 to +100

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Gate artifact publication on release gtests

When either new release job fails after the existing sanitizer dependencies complete, build-artifact can still run because its DAG needs list names only the four ASan/TSan jobs; deploy-artifact then needs only build-artifact and the native builds. Thus an O3-only failure—the precise case these jobs are meant to catch—can still publish the affected artifact before the pipeline turns red. Add both release jobs to the publication gate.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Confirmed and fixed in 691178f.

Checked it against the config rather than taking it on trust: build-artifact's needs block named exactly the four sanitizer jobs, and because that is an explicit DAG rather than a stage dependency, it starts as soon as those pass. The release jobs would have run alongside publication instead of ahead of it — so an -O3-only failure would still have turned the pipeline red, but only after the affected artifact was built, and deploy-artifact needs build-artifact. That would have left these jobs advisory, which defeats the point of adding them.

Both are now in the gate:

gtest-asan-amd64      optional=false
gtest-tsan-amd64      optional=true
gtest-asan-arm64      optional=false
gtest-tsan-arm64      optional=true
gtest-release-amd64   optional=false   <- added
gtest-release-arm64   optional=false   <- added

Two notes on the shape of the fix:

Non-optional, matching the ASan entries rather than the TSan ones. All six jobs come from .sanitizer_job and share its rules, so whenever build-artifact exists they exist too. optional: true would tolerate a future rules change, but it would also let the coverage disappear silently — the same invisibility this PR is closing — so a loud failure is the better mode here.

Only build-artifact needed changing; deploy-artifact names it in its own needs, so it is gated transitively. Verified rather than assumed.

tags: [ "arch:amd64" ]
image: $BUILD_IMAGE_X64
variables:
SANITIZER_CONFIG: Release
SANITIZER_LC: release

gtest-release-arm64:
extends: .sanitizer_job
tags: [ "arch:arm64" ]
image: $BUILD_IMAGE_ARM64
variables:
SANITIZER_CONFIG: Release
SANITIZER_LC: release

gtest-tsan-arm64:
extends: .sanitizer_job
# docker-in-docker:arm64 = EC2 VM. sysctl works directly.
Expand Down
Loading