Spoc 374: Add 'make installcheck' a Spock standard workflow#321
Merged
mason-sharp merged 2 commits intomainfrom Feb 5, 2026
Merged
Spoc 374: Add 'make installcheck' a Spock standard workflow#321mason-sharp merged 2 commits intomainfrom
mason-sharp merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow to run three-node Spock installcheck tests on PostgreSQL 15–18, updates Docker build/compose and container entrypoint logging, and adds a script to run pg_regress with artifact collection and failure-diff output. Changes
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/installcheck.yml:
- Around line 100-113: The workflow step "Collect installcheck artifacts"
attempts to copy regression.diffs unconditionally which can fail when the file
is absent; update the step to guard that copy by first checking for the file on
the container (e.g., run a short docker compose exec or docker compose run
command like "test -e /home/pgedge/installcheck-output/regression.diffs" and
only run "docker compose cp
pgedge-n1:/home/pgedge/installcheck-output/regression.diffs installcheck-logs/"
if present) or simply append "|| true" to the docker compose cp for
regression.diffs to make it non-fatal, keeping the other guarded copies
(regression.out, results) as-is.
- Around line 67-98: The step currently masks the exit code from "docker compose
exec -T pgedge-n1 bash -c '~/run-installcheck.sh' | tee installcheck_output.txt
|| true"; enable pipefail (set -o pipefail) at the top of the run block, run the
docker compose exec pipeline, then capture the producer's exit code via
PIPESTATUS[0] (e.g. DOCKER_EXIT=${PIPESTATUS[0]}) and fail the job or set
test_result=failed unless DOCKER_EXIT is 0 (or you still allow failures only for
security_label by combining this with the existing FAILED_TESTS check); update
the logic around FAILED_TESTS and the exit handling so the step exits non-zero
or writes test_result=failed when DOCKER_EXIT is non-zero and not accounted for
by ignored security_label failures.
In `@tests/docker/Dockerfile-step-1.el9`:
- Around line 126-137: The tag-selection pipeline that sets LATEST_TAG can pick
up prerelease tags (RC/BETA); update the pipeline that computes LATEST_TAG (the
series using git ls-remote | grep | sed | tr | sort | tail | tr) to explicitly
exclude tags containing _RC or _BETA (e.g., add a negative grep/filter for
(_RC|_BETA)) and then add a defensive test for non-empty LATEST_TAG before
running git clone (fail fast with an error/log if LATEST_TAG is empty);
reference the LATEST_TAG variable and the clone step that uses --branch
"${LATEST_TAG}" so you ensure only stable tags matching PGVER are cloned.
🧹 Nitpick comments (2)
tests/docker/entrypoint.sh (1)
66-69: Clarify the temporary logging override comment.
Line 67’s “To be changed on purpose” is vague; consider stating the reason (e.g., reduce installcheck noise) and when it should be reverted.tests/docker/run-installcheck.sh (1)
7-23: Fail fast on missing env and propagate early errors.
Lines 7–23 rely on PG_CONFIG/PG_SRCDIR/PGPORT/PGUSER being set; missing values produce confusing failures later. Consider enabling strict mode after sourcing and validating required variables.Proposed fix
source "${HOME}/.bashrc" + +set -euo pipefail + +: "${PG_CONFIG:?PG_CONFIG must be set}" +: "${PG_SRCDIR:?PG_SRCDIR must be set}" +: "${PGPORT:?PGPORT must be set}" +: "${PGUSER:?PGUSER must be set}"
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/installcheck.yml:
- Around line 44-74: Quote all instances of ${GITHUB_WORKSPACE} (e.g., change cd
${GITHUB_WORKSPACE}/tests/docker/ to cd "${GITHUB_WORKSPACE}/tests/docker/") and
any other unquoted expansions in the workflow to avoid word-splitting (also the
occurrences in sed commands and the docker compose up invocation), and replace
the tilde-in-quotes usage in the exec command (docker compose exec -T pgedge-n1
bash -c '~/run-installcheck.sh') with a dollar-home expansion (bash -c
"$HOME/run-installcheck.sh") so tilde expansion warnings are removed while
preserving behavior.
In `@tests/docker/run-installcheck.sh`:
- Around line 9-23: Add upfront validation for the other required PG_* env
variables before they are used: check PG_CONFIG, PG_SRCDIR, PGPORT, and PGUSER
(in addition to PGDATABASE) at the top of the script and fail fast with a clear
message and non-zero exit if any are unset; perform these checks before calling
"${PG_CONFIG}" or referencing PG_SRCDIR so functions/variables like BINDIR,
LIBDIR, PG_REGRESS, INPUTDIR, and SCHEDULE are not computed with missing inputs.
🧹 Nitpick comments (1)
tests/docker/run-installcheck.sh (1)
26-42: Quote paths/vars inpg_regressinvocation.Unquoted expansions can lead to word-splitting/globbing if any path contains spaces. Easy hardening.
Suggested fix
OUTPUTDIR="/home/pgedge/installcheck-output" -mkdir -p ${OUTPUTDIR} +mkdir -p "${OUTPUTDIR}" -${PG_REGRESS} \ - --dlpath=${PG_SRCDIR}/src/test/regress/ \ - --inputdir=${INPUTDIR} \ - --outputdir=${OUTPUTDIR} \ - --bindir=${BINDIR} \ - --schedule=${SCHEDULE} \ - --dbname=${PGDATABASE} \ + "${PG_REGRESS}" \ + --dlpath="${PG_SRCDIR}/src/test/regress/" \ + --inputdir="${INPUTDIR}" \ + --outputdir="${OUTPUTDIR}" \ + --bindir="${BINDIR}" \ + --schedule="${SCHEDULE}" \ + --dbname="${PGDATABASE}" \ --use-existing \ --host=/tmp \ - --port=${PGPORT} \ - --user=${PGUSER} + --port="${PGPORT}" \ + --user="${PGUSER}"
Add GitHub Actions workflow to run regression tests on a Docker-based three-node Spock cluster. Features: - Collect PostgreSQL logs from all nodes (n1, n2, n3) for debugging - Collect regression output (diffs, out, results) from n1 where tests run - Skip security_label test which cannot pass without PostgreSQL core changes
1. Use PG_SRCDIR consistently in run-installcheck.sh - Remove redundant local PG_SRC_DIR variable - Use PG_SRCDIR (set in .bashrc) throughout the script 2. Guard regression.diffs copy in workflow - Add || true since this file only exists on test failures - Prevents artifact collection from failing on successful runs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
danolivo
force-pushed
the
spoc-374
branch
3 times, most recently
from
cac034a to
1d74563
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/workflows/installcheck.yml:
- Around line 69-76: The step currently exits on a failing pipeline before
DOCKER_EXIT=${PIPESTATUS[0]} runs; change the script to temporarily disable
immediate exit around the test pipeline so the exit code can be captured. For
example, before running the pipeline for "docker compose exec -T pgedge-n1 bash
-c \"$HOME/run-installcheck.sh\" | tee installcheck_output.txt" do a set +e, run
the pipeline, immediately assign DOCKER_EXIT=${PIPESTATUS[0]}, then restore set
-e (or set -o errexit) so subsequent error-handling logic runs; this ensures
PIPESTATUS is captured even if the docker command fails.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
installcheckworkflow that runs standard PostgreSQL regression tests on a Docker-based three-node Spock cluster across PG 15-18. The workflow builds the cluster, runspg_regresson node n1, collects logs from all nodes, and uploads them as artifacts. Thesecurity_labeltest is excluded (requires PostgreSQL core changes).run-installcheck.shscript that invokespg_regressagainst the existing cluster using the standardparallel_schedule, outputting results to a dedicated directory for artifact collection.DBUSERandDBNAMEbuild args throughdocker-compose.ymlto allow overriding at build time (needed for regression test defaults)PG_SRCDIRin.bashrcso the installcheck script can locate the PostgreSQL source treeBASE_IMAGEtoghcr.io/pgedge/base-test-image:latestinstead of local taglog_statement = 'none'toentrypoint.shPostgreSQL config