Conversation
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
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow to run three-node Spock installcheck tests across PostgreSQL 15–18, updates Docker build/configuration and container entrypoint logging, and adds a script to run PostgreSQL regression tests via pg_regress with artifact collection and failure diff output. Changes
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}"
Comment on lines
+67
to
+98
| - name: Run installcheck on node n1 | ||
| id: installcheck | ||
| run: | | ||
| cd ${GITHUB_WORKSPACE}/tests/docker/ | ||
| # Run installcheck on n1 and capture output | ||
| docker compose exec -T pgedge-n1 bash -c '~/run-installcheck.sh' \ | ||
| | tee installcheck_output.txt || true | ||
| # Analyze the output for test failures | ||
| echo "Analyzing test results..." | ||
| # Check for 'exited with exit code' which indicates a crash or abnormal termination | ||
| if grep -q "exited with exit code" installcheck_output.txt; then | ||
| echo "ERROR: Found 'exited with exit code' in output - indicates abnormal termination" | ||
| echo "test_result=failed" >> $GITHUB_OUTPUT | ||
| exit 0 | ||
| fi | ||
| # Check for 'not ok' test results, excluding 'security_label' | ||
| # Extract all 'not ok' lines and filter out security_label | ||
| FAILED_TESTS=$(grep "not ok" installcheck_output.txt | grep -v "security_label" || true) | ||
| if [ -n "$FAILED_TESTS" ]; then | ||
| echo "ERROR: Found failing tests (excluding security_label):" | ||
| echo "$FAILED_TESTS" | ||
| echo "test_result=failed" >> $GITHUB_OUTPUT | ||
| else | ||
| echo "All tests passed (security_label failures are expected and ignored)" | ||
| echo "test_result=passed" >> $GITHUB_OUTPUT | ||
| fi | ||
| timeout-minutes: 30 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/installcheck.yml | sed -n '60,105p'Repository: pgEdge/spock
Length of output: 2273
Preserve docker compose exec exit status to avoid false passes.
The pipeline docker compose exec ... | tee installcheck_output.txt || true masks the exit status of the docker compose exec command itself. If the installcheck script fails but produces no "exited with exit code" or "not ok" strings, the step incorrectly passes. Use set -o pipefail and capture ${PIPESTATUS[0]} to detect unexpected failures, failing unless the exit status is zero or only security_label tests are affected.
Proposed fix
- docker compose exec -T pgedge-n1 bash -c '~/run-installcheck.sh' \
- | tee installcheck_output.txt || true
+ set -o pipefail
+ docker compose exec -T pgedge-n1 bash -c '~/run-installcheck.sh' \
+ | tee installcheck_output.txt
+ INSTALLCHECK_STATUS=${PIPESTATUS[0]}
@@
- if [ -n "$FAILED_TESTS" ]; then
+ if [ -n "$FAILED_TESTS" ]; then
echo "ERROR: Found failing tests (excluding security_label):"
echo "$FAILED_TESTS"
echo "test_result=failed" >> $GITHUB_OUTPUT
else
- echo "All tests passed (security_label failures are expected and ignored)"
- echo "test_result=passed" >> $GITHUB_OUTPUT
+ if [ "$INSTALLCHECK_STATUS" -ne 0 ] && ! grep -q "security_label" installcheck_output.txt; then
+ echo "ERROR: installcheck command failed unexpectedly"
+ echo "test_result=failed" >> $GITHUB_OUTPUT
+ else
+ echo "All tests passed (security_label failures are expected and ignored)"
+ echo "test_result=passed" >> $GITHUB_OUTPUT
+ fi
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - name: Run installcheck on node n1 | |
| id: installcheck | |
| run: | | |
| cd ${GITHUB_WORKSPACE}/tests/docker/ | |
| # Run installcheck on n1 and capture output | |
| docker compose exec -T pgedge-n1 bash -c '~/run-installcheck.sh' \ | |
| | tee installcheck_output.txt || true | |
| # Analyze the output for test failures | |
| echo "Analyzing test results..." | |
| # Check for 'exited with exit code' which indicates a crash or abnormal termination | |
| if grep -q "exited with exit code" installcheck_output.txt; then | |
| echo "ERROR: Found 'exited with exit code' in output - indicates abnormal termination" | |
| echo "test_result=failed" >> $GITHUB_OUTPUT | |
| exit 0 | |
| fi | |
| # Check for 'not ok' test results, excluding 'security_label' | |
| # Extract all 'not ok' lines and filter out security_label | |
| FAILED_TESTS=$(grep "not ok" installcheck_output.txt | grep -v "security_label" || true) | |
| if [ -n "$FAILED_TESTS" ]; then | |
| echo "ERROR: Found failing tests (excluding security_label):" | |
| echo "$FAILED_TESTS" | |
| echo "test_result=failed" >> $GITHUB_OUTPUT | |
| else | |
| echo "All tests passed (security_label failures are expected and ignored)" | |
| echo "test_result=passed" >> $GITHUB_OUTPUT | |
| fi | |
| timeout-minutes: 30 | |
| - name: Run installcheck on node n1 | |
| id: installcheck | |
| run: | | |
| cd ${GITHUB_WORKSPACE}/tests/docker/ | |
| # Run installcheck on n1 and capture output | |
| set -o pipefail | |
| docker compose exec -T pgedge-n1 bash -c '~/run-installcheck.sh' \ | |
| | tee installcheck_output.txt | |
| INSTALLCHECK_STATUS=${PIPESTATUS[0]} | |
| # Analyze the output for test failures | |
| echo "Analyzing test results..." | |
| # Check for 'exited with exit code' which indicates a crash or abnormal termination | |
| if grep -q "exited with exit code" installcheck_output.txt; then | |
| echo "ERROR: Found 'exited with exit code' in output - indicates abnormal termination" | |
| echo "test_result=failed" >> $GITHUB_OUTPUT | |
| exit 0 | |
| fi | |
| # Check for 'not ok' test results, excluding 'security_label' | |
| # Extract all 'not ok' lines and filter out security_label | |
| FAILED_TESTS=$(grep "not ok" installcheck_output.txt | grep -v "security_label" || true) | |
| if [ -n "$FAILED_TESTS" ]; then | |
| echo "ERROR: Found failing tests (excluding security_label):" | |
| echo "$FAILED_TESTS" | |
| echo "test_result=failed" >> $GITHUB_OUTPUT | |
| else | |
| if [ "$INSTALLCHECK_STATUS" -ne 0 ] && ! grep -q "security_label" installcheck_output.txt; then | |
| echo "ERROR: installcheck command failed unexpectedly" | |
| echo "test_result=failed" >> $GITHUB_OUTPUT | |
| else | |
| echo "All tests passed (security_label failures are expected and ignored)" | |
| echo "test_result=passed" >> $GITHUB_OUTPUT | |
| fi | |
| fi | |
| timeout-minutes: 30 |
🤖 Prompt for AI Agents
In @.github/workflows/installcheck.yml around lines 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.
Comment on lines
126
to
+137
| RUN set -eux && \ | ||
| LATEST_TAG=$(git ls-remote --tags https://github.com/postgres/postgres.git | \ | ||
| grep "refs/tags/REL_${PGVER}_" | \ | ||
| sed 's|.*refs/tags/||' | \ | ||
| tr '_' '.' | \ | ||
| sort -V | \ | ||
| tail -n 1 | \ | ||
| tr '.' '_') && \ | ||
| echo "Cloning PostgreSQL tag: ${LATEST_TAG}" && \ | ||
| git clone --branch "${LATEST_TAG}" --depth 1 \ | ||
| https://github.com/postgres/postgres.git /home/pgedge/postgres | ||
| LATEST_TAG=$(git ls-remote --tags https://github.com/postgres/postgres.git | \ | ||
| grep "refs/tags/REL_${PGVER}_" | \ | ||
| sed 's|.*refs/tags/||' | \ | ||
| tr '_' '.' | \ | ||
| sort -V | \ | ||
| tail -n 1 | \ | ||
| tr '.' '_') && \ | ||
| echo "Cloning PostgreSQL tag: ${LATEST_TAG}" && \ | ||
| git clone --branch "${LATEST_TAG}" --depth 1 \ | ||
| https://github.com/postgres/postgres.git /home/pgedge/postgres && \ | ||
| echo "export PG_SRCDIR=/home/pgedge/postgres" >> /home/pgedge/.bashrc |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify git ls-remote --tags behavior with PostgreSQL repository
echo "=== Testing git ls-remote --tags output format ==="
git ls-remote --tags https://github.com/postgres/postgres.git | grep -E "REL_[0-9]+_" | head -20Repository: pgEdge/spock
Length of output: 1320
🏁 Script executed:
#!/bin/bash
# Check for deref entries and RC/BETA tags in git ls-remote output
echo "=== Checking for deref entries (^{}) ==="
git ls-remote --tags https://github.com/postgres/postgres.git | grep '\^{}' | head -20
echo ""
echo "=== Checking for RC/BETA tags ==="
git ls-remote --tags https://github.com/postgres/postgres.git | grep -E '_RC|_BETA' | head -20
echo ""
echo "=== Checking PGVER value in Dockerfile ==="
cd /tmp && git clone https://github.com/pgEdge/spock.git 2>/dev/null && cat spock/tests/docker/Dockerfile-step-1.el9 | grep -E 'PGVER|ARG.*PGVER' | head -10Repository: pgEdge/spock
Length of output: 1919
🏁 Script executed:
#!/bin/bash
# Check for REL_17 tags specifically (the PGVER used in the Dockerfile)
echo "=== All REL_17 tags ==="
git ls-remote --tags https://github.com/postgres/postgres.git | grep 'refs/tags/REL_17' | head -30
echo ""
echo "=== Testing current pipeline logic with REL_17 ==="
# Simulate the current code with PGVER=17
PGVER=17
echo "Current code simulation:"
git ls-remote --tags https://github.com/postgres/postgres.git | \
grep "refs/tags/REL_${PGVER}_" | \
sed 's|.*refs/tags/||' | \
tr '_' '.' | \
sort -V | \
tail -n 5
echo ""
echo "=== Testing proposed fix logic with REL_17 ==="
echo "Proposed fix simulation:"
git ls-remote --tags https://github.com/postgres/postgres.git | \
sed -n 's|.*refs/tags/||p' | \
grep -E "^REL_${PGVER}_" | \
grep -vE '(_RC|_BETA)' | \
grep -v '\^{}' | \
sort -V | \
tail -n 5Repository: pgEdge/spock
Length of output: 1053
🏁 Script executed:
#!/bin/bash
# Final verification: check if ^{} entries appear in raw git ls-remote output
echo "=== Full git ls-remote --tags output (checking for ^{} deref entries) ==="
git ls-remote --tags https://github.com/postgres/postgres.git | head -50
echo ""
echo "=== Explicit check for lines containing ^ ==="
git ls-remote --tags https://github.com/postgres/postgres.git | grep '\^'Repository: pgEdge/spock
Length of output: 3203
Explicitly filter RC/BETA tags when selecting LATEST_TAG.
The current pipeline can select prerelease tags (RC, BETA) because git ls-remote --tags returns them and the grep filter doesn't exclude them. While sort -V currently prevents selection by chance (numeric version > letter), this is fragile and unreliable. The fix should explicitly filter out (_RC|_BETA) tags and add a defensive test -n check.
Proposed fix
-LATEST_TAG=$(git ls-remote --tags https://github.com/postgres/postgres.git | \
- grep "refs/tags/REL_${PGVER}_" | \
- sed 's|.*refs/tags/||' | \
- tr '_' '.' | \
- sort -V | \
- tail -n 1 | \
- tr '.' '_') && \
+LATEST_TAG=$(git ls-remote --tags https://github.com/postgres/postgres.git | \
+ sed -n 's|.*refs/tags/||p' | \
+ grep -E "^REL_${PGVER}_" | \
+ grep -vE '(_RC|_BETA)' | \
+ sort -V | \
+ tail -n 1) && \
+test -n "${LATEST_TAG}" && \
echo "Cloning PostgreSQL tag: ${LATEST_TAG}" && \
git clone --branch "${LATEST_TAG}" --depth 1 \
https://github.com/postgres/postgres.git /home/pgedge/postgres && \🤖 Prompt for AI Agents
In `@tests/docker/Dockerfile-step-1.el9` around lines 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.
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>
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}"
Comment on lines
+44
to
+74
| run: | | ||
| sudo chmod -R a+w ${GITHUB_WORKSPACE} | ||
|
|
||
| - name: Set up Docker | ||
| uses: docker/setup-buildx-action@e468171a9de216ec08956ac3ada2f0791b6bd435 # v3 | ||
|
|
||
| - name: Set up docker compose | ||
| uses: docker/setup-compose-action@364cc21a5de5b1ee4a7f5f9d3fa374ce0ccde746 # v1 | ||
| with: | ||
| version: latest | ||
|
|
||
| - name: Start docker cluster | ||
| run: | | ||
| cd ${GITHUB_WORKSPACE}/tests/docker/ | ||
| # To minimize regression tests difference, override pgedge.env with | ||
| # standard PostgreSQL regression test defaults | ||
| sed -i 's/^DBUSER=.*/DBUSER=regression/' pgedge.env | ||
| sed -i 's/^DBNAME=.*/DBNAME=regression/' pgedge.env | ||
| # Build and start the cluster | ||
| PGVER=${{ matrix.pgver }} DBUSER=regression DBNAME=regression \ | ||
| docker compose up --build --wait -d | ||
| timeout-minutes: 20 | ||
|
|
||
| - name: Run installcheck on node n1 | ||
| id: installcheck | ||
| run: | | ||
| cd ${GITHUB_WORKSPACE}/tests/docker/ | ||
|
|
||
| # Run installcheck on n1 and capture output | ||
| docker compose exec -T pgedge-n1 bash -c '~/run-installcheck.sh' \ | ||
| | tee installcheck_output.txt || true |
There was a problem hiding this comment.
Address shellcheck SC2086/SC2088: quote vars and avoid tilde-in-quotes.
actionlint reports word-splitting risks on unquoted ${GITHUB_WORKSPACE} and a tilde expansion warning in the bash -c string. Quoting and using inner $HOME expansion keeps behavior correct and silences the warnings.
Suggested fix
- sudo chmod -R a+w ${GITHUB_WORKSPACE}
+ sudo chmod -R a+w "${GITHUB_WORKSPACE}"
- cd ${GITHUB_WORKSPACE}/tests/docker/
+ cd "${GITHUB_WORKSPACE}/tests/docker/"
- docker compose exec -T pgedge-n1 bash -c '~/run-installcheck.sh' \
+ docker compose exec -T pgedge-n1 bash -c 'exec "$HOME/run-installcheck.sh"' \
- cd ${GITHUB_WORKSPACE}/tests/docker/
+ cd "${GITHUB_WORKSPACE}/tests/docker/"
- cd ${GITHUB_WORKSPACE}/tests/docker/
+ cd "${GITHUB_WORKSPACE}/tests/docker/"Also applies to: 102-137
🧰 Tools
🪛 actionlint (1.7.10)
[error] 44-44: shellcheck reported issue in this script: SC2086:info:1:19: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 56-56: shellcheck reported issue in this script: SC2086:info:1:4: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 69-69: shellcheck reported issue in this script: SC2086:info:13:32: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 69-69: shellcheck reported issue in this script: SC2086:info:1:4: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 69-69: shellcheck reported issue in this script: SC2086:info:24:32: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 69-69: shellcheck reported issue in this script: SC2086:info:27:32: Double quote to prevent globbing and word splitting
(shellcheck)
[error] 69-69: shellcheck reported issue in this script: SC2088:warning:4:42: Tilde does not expand in quotes. Use $HOME
(shellcheck)
🤖 Prompt for AI Agents
In @.github/workflows/installcheck.yml around lines 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.
Comment on lines
+9
to
+23
| # PGDATABASE should be previously set in the environment | ||
| if [ -z "${PGDATABASE}" ]; then | ||
| echo "The PGDATABASE environment variable must be set before running this command" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Get paths from pg_config | ||
| BINDIR="$(${PG_CONFIG} --bindir)" | ||
| LIBDIR="$(${PG_CONFIG} --libdir)" | ||
| PG_REGRESS="${LIBDIR}/postgresql/pgxs/src/test/regress/pg_regress" | ||
|
|
||
| # Standard PostgreSQL regression test directory (from source) | ||
| # PG_SRCDIR is set in .bashrc (sourced above) | ||
| INPUTDIR="${PG_SRCDIR}/src/test/regress" | ||
| SCHEDULE="${INPUTDIR}/parallel_schedule" |
There was a problem hiding this comment.
Fail fast when required PG_ variables are missing.*
Only PGDATABASE is validated, but PG_CONFIG, PG_SRCDIR, PGPORT, and PGUSER are used later. A simple upfront check avoids cryptic pg_config/path errors.
Suggested fix
# PGDATABASE should be previously set in the environment
-if [ -z "${PGDATABASE}" ]; then
- echo "The PGDATABASE environment variable must be set before running this command"
- exit 1
-fi
+for v in PGDATABASE PG_CONFIG PG_SRCDIR PGPORT PGUSER; do
+ if [ -z "${!v}" ]; then
+ echo "The ${v} environment variable must be set before running this command"
+ exit 1
+ fi
+done📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # PGDATABASE should be previously set in the environment | |
| if [ -z "${PGDATABASE}" ]; then | |
| echo "The PGDATABASE environment variable must be set before running this command" | |
| exit 1 | |
| fi | |
| # Get paths from pg_config | |
| BINDIR="$(${PG_CONFIG} --bindir)" | |
| LIBDIR="$(${PG_CONFIG} --libdir)" | |
| PG_REGRESS="${LIBDIR}/postgresql/pgxs/src/test/regress/pg_regress" | |
| # Standard PostgreSQL regression test directory (from source) | |
| # PG_SRCDIR is set in .bashrc (sourced above) | |
| INPUTDIR="${PG_SRCDIR}/src/test/regress" | |
| SCHEDULE="${INPUTDIR}/parallel_schedule" | |
| # PGDATABASE should be previously set in the environment | |
| for v in PGDATABASE PG_CONFIG PG_SRCDIR PGPORT PGUSER; do | |
| if [ -z "${!v}" ]; then | |
| echo "The ${v} environment variable must be set before running this command" | |
| exit 1 | |
| fi | |
| done | |
| # Get paths from pg_config | |
| BINDIR="$(${PG_CONFIG} --bindir)" | |
| LIBDIR="$(${PG_CONFIG} --libdir)" | |
| PG_REGRESS="${LIBDIR}/postgresql/pgxs/src/test/regress/pg_regress" | |
| # Standard PostgreSQL regression test directory (from source) | |
| # PG_SRCDIR is set in .bashrc (sourced above) | |
| INPUTDIR="${PG_SRCDIR}/src/test/regress" | |
| SCHEDULE="${INPUTDIR}/parallel_schedule" |
🤖 Prompt for AI Agents
In `@tests/docker/run-installcheck.sh` around lines 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.
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
2 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.
No description provided.