Skip to content

[TASK] DPL-204: runTests.sh follow-ups - #72

Merged
sbuerk merged 2 commits into
1from
task/dpl-204-ci-followups-1
Jul 31, 2026
Merged

[TASK] DPL-204: runTests.sh follow-ups#72
sbuerk merged 2 commits into
1from
task/dpl-204-ci-followups-1

Conversation

@sbuerk

@sbuerk sbuerk commented Jul 31, 2026

Copy link
Copy Markdown
Member

Follow-up work left over from the CI docker adoption (WVP-106) for
branch 1. Everything here is pre-existing — none of it was caused by
that adoption.

Addresses DPL-204 (T3, T4). DPL-214 has no work on this branch
see below. The companion pull request for main carries T1 and the 8
label edits.

This branch touches Build/Scripts/runTests.sh only. No workflow file
is changed at all.

Commits

Commit Issue What
[TASK] DPL-204: Own the sqlite tmpfs from the host DPL-204 T3 HOST_GID, TMPFS_MOUNT_OPTIONS per container binary, used on the sqlite --tmpfs
[TASK] DPL-204: Revive two unused harness variables DPL-204 T4 CI_PARAMS assigned; renderDocumentation now uses DOCUMENTATION_COMMON_PARAMS

Which tasks applied

  • T1 — MySQL-named steps ran MariaDB: not fixed here, deliberately.
    The 4 mislabelled steps exist on this branch too (2 in
    testcore12.yml, 2 in testcore13.yml), but the entire testsuite:
    job is commented out in both files — job header, matrix, permissions
    and every step. Parsing either file yields only the code-quality
    job, so this branch runs zero functional steps. The defect is
    dormant, not live. Un-commenting the block to fix it would re-arm a
    whole test job, which is exactly what the "no job added, removed or
    reordered" constraint rules out. Fix it there if the job is ever
    revived.
  • T2 — hardcoded -it in renderDocumentation: does not apply, as
    the issue already states. The only -it on this branch is the
    legitimate CONTAINER_INTERACTIVE="-it --init", which is emptied when
    CI=true. Left alone.
  • T3 — --user without a gid: applied, byte-identical to main.
    ${USERSET} is deliberately untouched; the mount options moved into
    TMPFS_MOUNT_OPTIONS, where docker adds uid/gid and podman does
    not need them. The existing mode=1777 is kept in both.
  • T4 — dead variables: applied, both halves. CI_PARAMS was
    referenced but never assigned, DOCUMENTATION_COMMON_PARAMS was built
    but never read.

Label changes (DPL-214)

None. DPL-214 is scoped to main only, and this branch has no live
functional step to relabel. The dormant steps inside the commented-out
testsuite job were left exactly as they are — editing commented lines
would only make a future revival look reviewed when it has not been.

The renderDocumentation difference from main

Worth a reviewer's eye, because this branch's line is not the one the
issue describes. On main, renderDocumentation used
CONTAINER_COMMON_PARAMS and therefore picked up --network,
--add-host and a stray -w. Here it built its own line instead:

${CONTAINER_BIN} run ${CONTAINER_INTERACTIVE} --pull always -v ${ROOT_DIR}:/project ${IMAGE_RSTRENDERING} --config=Documentation

so it ran without --rm and, under docker, as root rather than as the
host user — which is how root-owned files end up in
Documentation-GENERATED-temp. Pointing it at
DOCUMENTATION_COMMON_PARAMS keeps the same
${CONTAINER_INTERACTIVE}, adds the missing --rm and ${USERSET},
and supplies the /project mount, so the repeated -v is dropped from
the line. --pull always and the image arguments are unchanged.

publish.yml renders the documentation into a release asset on this
branch, so this path is exercised.

Corrections to the issue text

  • DPL-204's T4 says renderDocumentation uses CONTAINER_COMMON_PARAMS
    and inherits --network, --add-host and a stray -w. True on
    main, not true on this branch — see above. The dead-variable
    conclusion holds; the stated consequence does not.
  • DPL-204's T4 says CI_PARAMS is referenced in the podman branch. It is
    referenced at 4 sites, and the mock-server run line is
    unconditional, so the missing assignment also affected the docker path.
  • DPL-204's T1 scope line is accurate: it already notes the 4 steps on
    1 are inside a commented-out block.

Verification

Static plus CI. The functional suites were not run locally — this is one
of 24 branches in the sweep, and each pull request's own CI exercises
them. On this branch there is in any case no functional CI job to run.

bash -n Build/Scripts/runTests.sh                      # OK
python3 -c "import glob,yaml;[yaml.safe_load(open(f)) for f in glob.glob('.github/workflows/*.yml')]"
git diff --name-only origin/1..HEAD
  Build/Scripts/runTests.sh

Structural comparison of every workflow against origin/1: all 3 files
identical in structure and byte-identical — git diff origin/1..HEAD -- .github/ is empty, which is the strongest possible statement that no
job, step, matrix entry, trigger or commented line moved.

Substantive property checks, deliberately not grepping for strings that
also occur in the inserted comments:

  • grep -E '^\s*TMPFS_MOUNT_OPTIONS="' → the two expected assignments
  • sqlite mount uses …functional-sqlite-dbs/:${TMPFS_MOUNT_OPTIONS}, no
    literal option list left on that line
  • grep -E '^CI_PARAMS="' → 1, grep -E '^HOST_GID=' → 1
  • renderDocumentation run line uses ${DOCUMENTATION_COMMON_PARAMS}
  • lines 1–44 of runTests.sh (the waitFor cap of 60 with its
    cleanUp; exit 1 abort, and handleDbmsOptions) byte-identical to
    origin/1

Acceptance

  • one pull request per affected branch — this one plus the main
    companion
  • diff touches only Build/Scripts/runTests.sh
  • no workflow job, step, matrix entry or trigger added, removed or
    reordered; no commented block un-commented
  • commit subjects [TAG] <ISSUE>: Subject ≤ 52 characters, bodies
    wrapped at 72
  • do-not-touch list respected: -b docker flags, the WVP-106 workflow
    header comment, the waitFor cap of 60 and its abort,
    CONTAINER_INTERACTIVE="-it --init", and the sqlite mode=1777
  • CI to confirm

sbuerk added 2 commits July 31, 2026 17:09
"${USERSET}" passes "--user ${HOST_UID}" without a group, so a docker
container runs as "uid=${HOST_UID} gid=0(root)". A "--tmpfs" is created
"root:root" and inherits the mode of its host mountpoint, which is 0755
at a CI umask of 0022 - group 0 gets "r-x" only, and the functional
sqlite suite fails with "unable to open database file".

The docker adoption worked around this by mounting the sqlite tmpfs
"mode=1777". That is correct and umask independent, but it treats the
one mount rather than the missing group.

The mount options move into "TMPFS_MOUNT_OPTIONS", assigned next to the
container parameters they belong to, so the two container binaries can
state what they actually need: docker adds "uid" and "gid" and keeps
"mode=1777", rootless podman maps the container root to the host user
and needs neither, keeping "mode=1777" for the rootful case.

"${USERSET}" is deliberately left alone. It is evaluated for both
container binaries, and appending a group there would change which host
group rootless podman maps the container to, which is a different
question from who owns a tmpfs.

The comment moves along with the options and now names the umask that
makes this visible: at the 0002 of a typical workstation the mountpoint
comes up 0775 and the defect cannot be reproduced at all.
Two variables were referenced or built but never took effect, and both
misled anyone reading the script.

"CI_PARAMS" is expanded into the podman container parameters and into
the mock server container - four sites, and the mock server one is
unconditional, so it also applies under docker - but was never
assigned. It is not a leftover though: the harnesses this one is
modelled on assign it as "CI_PARAMS="${CI_PARAMS:-}"" and treat it as
an escape hatch a caller can export to inject additional container
flags. Only that assignment was missing here, so it is added rather
than the references removed.

"DOCUMENTATION_COMMON_PARAMS" was assigned for both container binaries
and then never used. "renderDocumentation" built its own line instead,
inlining "${CONTAINER_INTERACTIVE}" and repeating the bind mount, and
so ran without "--rm" and, under docker, as root rather than as the
host user - which is why rendering can leave root owned files behind
in "Documentation-GENERATED-temp".

It now uses the parameters that were built for it. They carry the same
"${CONTAINER_INTERACTIVE}" the line had, add the "--rm" and the
"${USERSET}" it was missing, and provide the "/project" mount, so the
mount is dropped from the line. "--pull always" and the image
arguments are unchanged. This also brings the call in line with
"main", where the same variable is wired up.
@sbuerk
sbuerk merged commit 3973c6f into 1 Jul 31, 2026
2 checks passed
@sbuerk
sbuerk deleted the task/dpl-204-ci-followups-1 branch July 31, 2026 15:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant