ci: run every test on every PR on self-hosted runners; drop testing.Short() - #422
Conversation
…Short() - delete all 413 testing.Short() gates and the scenariosSkipIfShort helper; -short is gone from ci.yml, scripts/test-mysql.sh (quick mode) and Makefile - ci.yml: one build-and-test job on runs-on: self-hosted running `go test -p=8 ./...` plus the conformance module, with a Trino oracle started on a random port; the tagged paren gates are sibling self-hosted jobs; bytebase/bytebase conventions (permissions, concurrency, cache: false) - container-tests.yml -> nightly.yml: schedule/dispatch only (oracle-ref, splittest, wide-N fuzz, new tidb-catalog-parity) - share one engine container per package: tidb/catalog now uses the shared TiDB (it booted mysql:8.0 per test before, 981s -> 69s); mssql/parser, mysql/parser and mariadb/parser share via sync.Once + reset; the mysql/catalog TestOracle_* family falls back to mysql:8.0/5.7 testcontainers when the fixed-port engines are absent (it never ran in CI before) - quarantine tidb/catalog's MySQL-shaped differential family (93 mismatches vs TiDB v8.5.5) behind TIDB_CATALOG_PARITY=1 and run it nightly - retry the first TiDB ping: the port listens before the server accepts connections Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c2dad92b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
- ci.yml is now a single `ci` job. A PR runs `go test` on what scripts/affected-packages.sh prints: the top-level directories it touched plus every directory whose packages or tests import one of them (from `go list`), or ./... when something outside a Go directory changed. Pushes to main run ./... . - Trino is a `services:` container (the runner waits for its health check) instead of start/stop steps. - the -tags=oracle PG/Redshift differentials run strictly, only when pg or redshift is affected; they take seconds, so defer mode and the artifact uploads go. - nightly.yml and paren-oracle.yml are deleted: the wide-N sweeps run in 25-40s and are covered by the PR gate; the two lanes that only ever failed there (oracle-ref, tidb parity) are follow-ups. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…unt=1 in CI Codex review on #422: with Short() gone, doris/starrocks/oracle and the MySQL catalog/validate/parser helpers failed a Docker-less laptop's `go test ./...`; they now follow the CI-fatal / local-skip convention the other helpers use. The self-hosted runners keep GOCACHE, so CI passes -count=1 to make sure every selected test executes rather than being served from the test cache. Per-package -timeout flags go; the 30-minute job timeout is the guard. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f302045004
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
grep -v exits 1 when every changed file is markdown, which set -e turned into a failed step (Codex review on #422). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a631d7584
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
The nightly-only sizes (fuzz N=10000, splittest N=10^6, S3 N=2000) take ~20 s on the runner, so the PR gate uses them. harness/googlesql-spanner is its own module and was never reached by ./...; its non-emulator tests now run alongside harness/conformance. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…dinary container tests They only need the PG 17 testcontainer the untagged pg/catalog tests already use, and run in ~10 s, so the tag was the -short idea in another form. They now run with the rest of pg/ and redshift/ (strict, wide-N). oracle_ref, googlesql_oracle and scriptdom keep their tags for stated reasons (open mismatches, emulator drift, .NET harness). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d649e7665f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Codex review on #422: the trino packages share the one Trino service and run stateful DDL against the same names, so they now run with -p=1 after the rest, as the old lane did. scripts/ holds the CI drivers (including the selector itself) and has one Go file, which made a script-only change look engine-scoped; it now maps to ./... . Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d490eec811
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…smatches
The oracle_ref tag and the ORACLE_PARSER_REF_CONTAINER/STRICT switches are
gone: the lane starts Oracle 23ai Free like every other container test
(DSN env vars still override) and runs with ./oracle/... in ~20 s.
The two mismatches that kept it red were not parser bugs:
- ORA-24344 ("a compilation error occurred while creating an object") is
Oracle accepting the DDL and creating an invalid object; the harness read
it as a syntax rejection (ref_030, CREATE TYPE BODY without its spec).
- compat_plsql_009/038/039 create deliberately broken triggers on t and
had no cleanup because the authors assumed Oracle rejects them; it
creates them invalid, so the next INSERT INTO t in TestOracleReference
hit ORA-04098. They now drop their trigger.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ET 8 The scriptdom tag is gone. The test skips locally when the .NET SDK is absent and fails in CI, where actions/setup-dotnet provides 8.0.x; the harness builds in ~10 s and the two tests run in ~8 s. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…offline The googlesql_oracle tag is gone. Every harness constructor now skips locally without SPANNER_EMULATOR_HOST and fails in CI, where ci.yml runs the Spanner emulator as a service pinned to 1.5.54, the build the recorded verdicts came from (sha256:caf1bd24; 1.5.57 drifts on two forms). The harness bootstrapped one fixed database, so parallel test binaries dropped each other's; it now uses a per-process database and drops it on exit. pg/catalog's shared PG helper panicked on startup failure; it records the error and skips locally / fails in CI like the redshift copy (Codex review). No test build tag remains in the repo. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
actions/setup-dotnet defaults to /usr/share/dotnet, which the non-root self-hosted runner cannot create. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6c7976ab2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
harness/mssql-scriptdom and harness/googlesql-spanner are exec'd by mssql/parser and googlesql tests, not imported, so go list cannot map a harness change to its callers (Codex review on #422). Treat harness/ like scripts/. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72947b7031
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
A test that needs an engine starts it, and an engine that cannot start fails the test. Gone: the CI-gated skip branches, the isCI helpers, the scenario Docker probe and SKIP_SCENARIO_TESTS, the last build tag on tools.go (hjson is imported directly), and every -short/tag mention in the docs. - Trino: trinooracle.ForTest uses $TRINO_ORACLE_URL / localhost:18080 when one answers, else a testcontainer once per test binary. - Spanner: googlesql/internal/spannertest.Host uses $SPANNER_EMULATOR_HOST when set, else an emulator testcontainer pinned to 1.5.54, exported through that variable so the harness subprocesses find it. - ScriptDom: the .NET harness builds and runs inside mcr.microsoft.com/dotnet/sdk:8.0 via docker run, so no host .NET; ci.yml drops actions/setup-dotnet. - Conformance: the smoke test runs fetch_corpus.sh itself; ci.yml drops the fetch step. Verified on this machine with no env vars, no .NET and no corpus: go test ./... green in 174 s, both nested modules green, no containers left behind. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…USER_ERRORS Codex review on #422: ORA-24344 also covers PL/SQL objects created with parse errors, so treating it as accepted turned ref_030 (a type body missing its END, expected reject) into a false match with omni's over-acceptance. The harness now asks USER_ERRORS: PLS-00103 keeps the rejection, anything else (undeclared identifier, body without spec) is semantic and counts as accepted. And the parser now requires END after the type body members, as Oracle does; the one unit test that relied on the lenient form used a body Oracle itself rejects and now uses a valid one. Also from the review: docs/ can hold test corpora (googlesql truth1), so the selector no longer ignores markdown; a docs-only change runs ./... . Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 21247817f0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
grep exits 1 when it selects nothing; GitHub's default bash -e does not set pipefail, but an explicit shell would, so guard both pipelines (Codex review on #422). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 participant
Summary
Removes the
testing.Short()concept, replaces three workflows and twenty jobs with one job on the org's self-hosted runners that runs the tests of the engines a PR touches (everything on pushes to main), and shares one engine container per package instead of one per test.Why
go test -short ./...in ci.yml skipped 413 gated tests in 100 files, and container-tests.yml re-ran them per engine through hand-written-runregexes. Nothing checked that the lanes covered everything, and they didn't:TestOracle_*, 5TestDeparse_Section_7_*, 1 shared-container test), 2 in mysql/parser, 1 in mysql/validate, 2 in doris/parser, the conformance corpus smoke testWhat changed
No more
Short()(172 files, −1,328 lines). Everyif testing.Short() { t.Skip }block and thescenariosSkipIfShorthelper are gone;-shortis gone from ci.yml,scripts/test-mysql.sh(thequickmode) and the Makefile. Docker is assumed everywhere: a test that needs an engine starts it, and an engine that cannot start fails the test. There is noCIenv switch, no Docker probe, no skip-when-unavailable branch left.Shared containers. One engine per package, reset between tests (the bytebase/bytebase testcontainer approach):
startTiDBForCatalog) + TiDB-flavoured reset, 69 sTestOracle_*oracle_fallback_test.go)CI: one workflow, one job (ci.yml).
runs-on: self-hosted, bytebase/bytebase conventions (read-only permissions, per-PR concurrency with cancel,cache: falsebecause the runner keeps GOCACHE/GOMODCACHE). A PR runsgo test -count=1on the packagesscripts/affected-packages.shprints: the top-level directories it touched plus every directory whose packages or tests import one of them, computed fromgo list, so nothing is hand-maintained (the engines are independent Go trees; onlymetadatais shared). Anything outside a Go directory (go.mod, proto, workflows, docs/, root files), under scripts/ or under harness/ (harnesses are exec'd, not imported) means./.... Pushes to main always run./.... Trino runs as aservices:container. The two nested modules (harness/conformance, harness/googlesql-spanner) run every time. The sweep sizes the old nightly used (fuzz N=10000, splittest N=10⁶, S3 N=2000) cost ~20 s, so every PR gets them. container-tests.yml, nightly.yml and paren-oracle.yml are deleted.No build tags, no env switches.
oracle,oracle_ref,googlesql_oracleandscriptdomare gone, and so areORACLE_PARSER_REF_CONTAINER,SKIP_SCENARIO_TESTS, theCI-gated skips and the Docker probe. Every differential provisions what it needs through Docker: PG 17, MySQL 5.7/8.0, MariaDB, TiDB, MSSQL, StarRocks, Doris and Oracle 23ai Free as testcontainers; Trino (trinooracle.ForTest) and the Spanner emulator (googlesql/internal/spannertest, pinned to 1.5.54, the build the recorded verdicts came from) as testcontainers unlessTRINO_ORACLE_URL/SPANNER_EMULATOR_HOSTpoint at a running one, which is what ci.yml'sservices:do; the ScriptDom harness builds and runs insidemcr.microsoft.com/dotnet/sdk:8.0, so no .NET install anywhere; the conformance smoke test fetches its corpus itself. Three harness bugs and one parser gap surfaced on the way and are fixed: the Oracle harness read ORA-24344 (object created with compilation errors) as a syntax rejection regardless of cause and now asks USER_ERRORS whether the compile error was a parse error (PLS-00103); it leaked three deliberately broken triggers between tests; the googlesql harness bootstrapped one fixed database so parallel test binaries dropped each other's; and omni accepted a CREATE TYPE BODY without its closing END, which Oracle rejects (ref_030) and omni now rejects too.Quarantine, not a flag. tidb/catalog's differential family (forked from mysql/catalog, MySQL-shaped expectations) fails 93 tests against real TiDB.
startContainerskips it with an explicit reason unlessTIDB_CATALOG_PARITY=1. Fixing the family is the follow-up.Verified locally (20 vCPU / 31 GB, Docker,
CI=true, Trino on a random port,-p=8)go test ./...: every package ok, on this machine and on the self-hosted pool (full suite about 3.5 minutes there, trino serialized)harness/conformance: ok with the fetched corpusRepo settings (done)
mainnow requires the singlecistatus check (the 18 old per-engine and(1.25)contexts are gone). The other open PRs need a merge from main to pick up the new workflow.🤖 Generated with Claude Code