Skip to content

Run Go binding tests under -race#266

Merged
George-Payne merged 1 commit into
mainfrom
bindings-race
Jul 6, 2026
Merged

Run Go binding tests under -race#266
George-Payne merged 1 commit into
mainfrom
bindings-race

Conversation

@George-Payne

Copy link
Copy Markdown
Member

Follow-up to #265: run the Go binding tests under -race, now that the checkptr-illegal handle conversion is gone.

  • bindings/go/justfile - -race added to the test recipe. CI inherits it via just _test -> bindings::test -> go::test, no workflow changes. Beyond data races, -race enables checkptr, so a future integer-handle-in-pointer-slot regression fatals deterministically at the conversion site instead of crashing the GC once in a blue moon (UI-1813). Measured cost: the suite goes from ~0.5s to ~1.6s, with race-instrumented artifacts cached by setup-go, so steady-state CI pays a couple of seconds.

Possible since #265 removed the uintptr->pointer handle conversion that
checkptr fataled on. Beyond data races, -race enables checkptr, turning
any future integer-handle-in-pointer-slot regression into a deterministic
fatal at the conversion site instead of a rare GC crash (UI-1813). The
suite goes from ~0.5s to ~1.6s; race-instrumented artifacts land in the
Go build cache, so CI steady state pays a couple of seconds.
@George-Payne George-Payne requested a review from a team as a code owner July 6, 2026 13:55
@George-Payne George-Payne added tooling suggestion for new or exsisting tools bindings-go Go bindings to the C runtime (gafferruntime module) labels Jul 6, 2026
@George-Payne George-Payne self-assigned this Jul 6, 2026
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Run Go binding tests with -race via justfile recipe

🧪 Tests ⚙️ Configuration changes 🕐 Less than 5 minutes

Grey Divider

AI Description

• Run Go bindings test suite with Go race detector enabled.
• Use -race to also enforce checkptr on FFI handle conversions (UI-1813).
• Keep CI wiring unchanged by inheriting the updated just test recipe.
Diagram

graph TD
  CI(["CI"]) --> J["just _test"] --> B["bindings::test"] --> T["go::test"] --> G["go test -race"]
Loading
High-Level Assessment

The chosen approach (enable -race directly in the bindings/go justfile test recipe) is the most direct and maintainable way to ensure both race detection and checkptr coverage run in CI without workflow changes. Alternatives like adding separate CI-only flags or duplicating targets add branching/maintenance with little benefit.

Files changed (1) +4 / -2

Tests (1) +4 / -2
justfileEnable -race for Go bindings tests +4/-2

Enable -race for Go bindings tests

• Updates the 'test' recipe to run 'go test' with '-race' while keeping the existing 'dev' tag and CGO enabled. Expands comments to document the additional checkptr/FFI safety value and the motivation (UI-1813).

bindings/go/justfile

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

@George-Payne George-Payne merged commit 9921213 into main Jul 6, 2026
3 checks passed
@George-Payne George-Payne deleted the bindings-race branch July 6, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bindings-go Go bindings to the C runtime (gafferruntime module) tooling suggestion for new or exsisting tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant