test/system: use a random port for the local Docker registry#1822
test/system: use a random port for the local Docker registry#1822Rolv-Apneseth wants to merge 1 commit into
Conversation
Hard-coding the port to 50000 can cause intermittent failures when the port happens to already be in use, and also prevents running the test suite more than once at the same time. Let Podman pick a random available port and persist it to a file so test processes in the same run can find it. Signed-off-by: Rolv Apneseth <rolv.apneseth@gmail.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the BATS helper script to dynamically assign and persist the Docker registry URI using a randomly assigned host port, rather than using a hardcoded port. This prevents port conflicts during parallel test execution. The review feedback points out that using run followed by assert_success inside _setup_docker_registry is a BATS anti-pattern and can lead to undefined behavior, suggesting executing the commands directly instead.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| run mkdir -p "$HOME"/.config/containers/certs.d/"${DOCKER_REG_URI}" | ||
| assert_success | ||
| run cp "${DOCKER_REG_CERTS_DIR}"/domain.crt "$HOME"/.config/containers/certs.d/"${DOCKER_REG_URI}"/domain.crt | ||
| assert_success |
There was a problem hiding this comment.
According to the repository's general rules, using run followed immediately by assert_success is an anti-pattern in BATS. Additionally, run should not be used inside setup_suite() or functions called by it (such as _setup_docker_registry()), because BATS variables like BATS_TEST_TMPDIR are not defined in that context, which can lead to undefined behavior. These commands should be executed directly instead.
mkdir -p "$HOME"/.config/containers/certs.d/"${DOCKER_REG_URI}"
cp "${DOCKER_REG_CERTS_DIR}"/domain.crt "$HOME"/.config/containers/certs.d/"${DOCKER_REG_URI}"/domain.crt
References
- In BATS (Bash Automated Testing System), avoid using 'run' followed immediately by 'assert_success' as it is considered an anti-pattern. Additionally, avoid using 'run' inside 'setup_suite()' or functions called by it, because BATS variables like 'BATS_TEST_TMPDIR' are not defined in that context, which can lead to undefined behavior.
There was a problem hiding this comment.
That's pre-existing code that I just moved, but that makes sense, I could make that change
1 participant
See #1817 (comment). I believe this should do the trick @debarshiray