Conversation
- Add support for MinIO endpoints using mc (MinIO Client) - Use mc for reliable file uploads when custom S3 endpoint is provided - Fallback to AWS Signature V4 for AWS S3 - Simplify directory path handling in script setup Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
There are multiple concrete runtime-breaking issues in the new external/docker workflow paths (compose invocation, invalid compose mount path, script/CLI typos, and container missing required tooling).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds support for running the existing Semaphore UI test suite against an already-running (“external”) Semaphore instance, including a Dockerized test-runner workflow and S3 publishing utilities, while also translating/reorganizing much of the test-environment documentation into English.
Changes:
- Add “external” execution mode support (stand config + Gradle tasks + run scripts/Taskfile entries) to run tests against a running Semaphore.
- Add a Docker-based test-runner (
Dockerfile,docker-compose.yml) plus scripts to run external tests and publish Allure results to S3. - Translate/move documentation (test-environment docs to English; research docs moved under
AGENTS/plans/).
File summaries
| File | Description |
|---|---|
| test-environment/upgrade-report.md | English translation of the upgrade incompatibility report. |
| test-environment/smoke-report.md | English translation of the local smoke report. |
| test-environment/README.md | English translation + updated guidance for profiles/CI. |
| test-environment/profiles/external/profile.yaml | Adds an external profile manifest (currently not runnable due to empty compose). |
| test-environment/profiles/external/compose.yml | Added as an external profile compose file (currently empty). |
| test-environment/profile | Switches compose invocation (currently to docker-compose). |
| test-environment/fixtures/bash/capture-output/normal.sh | Adds bash fixture for stdout/stderr capture tests. |
| test-environment/fixtures/bash/capture-output/background.sh | Adds bash fixture for bounded output draining tests. |
| test-environment/fixtures/ansible/README.md | English translation of the trusted Ansible fixture description. |
| test-environment/fixtures/ansible/long-running.yml | Changes stoppable task fixture pause implementation. |
| test-environment/configuration-testing-overview.md | English translation of configuration testing strategy doc. |
| test-environment/compose.base.yml | Updates fixture-init bind mount path (currently incorrect). |
| test-environment/api-map.md | English translation of the API map and discrepancies list. |
| Taskfile.yml | Adds task shortcuts for external testing, profiles, and S3 publishing. |
| src/test/resources/stands/external/stand.properties | Adds external stand defaults/config documentation. |
| src/main/java/io/bookwright/fixtures/semaphore/SemaphoreFixtures.java | Switches fixtures to use a GitHub repo and updated playbook/script paths. |
| semaphore-ui-testing-assessment-plan.md | Removed from root (moved under AGENTS/plans/). |
| semaphore-testing-component-map.md | Removed from root (moved under AGENTS/plans/). |
| semaphore-issues-problem-register.md | Removed from root (moved under AGENTS/plans/). |
| scripts/run-external-tests.sh | Adds a CLI script to run Gradle external tests against a running Semaphore. |
| scripts/publish-to-s3.sh | Adds S3 publishing script for Allure single-file HTML output. |
| run.sh | Adds a command router wrapper for local/external runs + publishing. |
| README.md | English rewrite of the repository README and updated run instructions. |
| Dockerfile | Adds a Ubuntu-based test-runner image (Java 21 + Docker CLI + compose + mc). |
| docker-compose.yml | Adds a test-runner service for executing tests in a container. |
| build.gradle.kts | Adds externalTest and externalSmokeTest Gradle test tasks. |
| AGENTS/plans/semaphore-ui-testing-assessment-plan.md | Adds the assessment plan under AGENTS/plans/ (English). |
| AGENTS/plans/semaphore-testing-component-map.md | Adds the component map under AGENTS/plans/ (English). |
| AGENTS/plans/semaphore-issues-problem-register.md | Adds the issue registry under AGENTS/plans/ (English). |
| .dockerignore | Adds Docker build context ignore rules. |
Review details
- Files reviewed: 26/30 changed files
- Comments generated: 9
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Create the entry point | ||
| RUN chmod +x /workspace/run-external-tests.sh 2>/dev/null || true && \ | ||
| chmod +x /workspace/publish-results-local.sh 2>/dev/null || true |
|
|
||
| test_external() { | ||
| local host=$(get_param "host" "$@" 2>/dev/null || echo "host.docker.internal") | ||
| local port=$(bget_param "port" "$@" 2>/dev/null || echo "3000") |
| @@ -62,17 +62,17 @@ public static SemaphoreFixtures from(MainConfig config, TestData data) { | |||
| "[local]\nlocalhost ansible_connection=local", | |||
| "static"), | |||
| new Templates( | |||
| new Template("bookwright-build-template-" + suffix, "smoke.yml", "ansible", ""), | |||
| new Template("bookwright-build-template-" + suffix, "test-environment/fixtures/ansible/smoke.yml", "ansible", ""), | |||
| new Template( | |||
| "bookwright-stoppable-template-" + suffix, "long-running.yml", "ansible", ""), | |||
| "bookwright-stoppable-template-" + suffix, "test-environment/fixtures/ansible/long-running.yml", "ansible", ""), | |||
| new Template( | |||
| "bookwright-shell-output-template-" + suffix, | |||
| "bash/capture-output/normal.sh", | |||
| "test-environment/fixtures/bash/capture-output/normal.sh", | |||
| "bash", | |||
| ""), | |||
| new Template( | |||
| "bookwright-background-shell-output-template-" + suffix, | |||
| "bash/capture-output/background.sh", | |||
| "test-environment/fixtures/bash/capture-output/background.sh", | |||
| "bash", | |||
| "")), | |||
| volumes: | ||
| - fixture-repository:/repository | ||
| - ./fixtures/ansible:/source:ro | ||
| - ./test-environment/fixtures/ansible:/source:ro |
| id: external | ||
| description: Test external semaphore instance with Semaphore API tests | ||
| compose_file: compose.yml | ||
| compose_project: external | ||
| readiness_url: http://localhost:3000/api/ping | ||
| stand: semaphore |
| # Core utilities | ||
| curl \ | ||
| wget \ | ||
| unzip \ | ||
| ca-certificates \ | ||
| \ | ||
| # For tests | ||
| bash \ | ||
| coreutils \ | ||
| \ |
| environment: | ||
| GRADLE_OPTS: -Xmx2g | ||
| API_BASE_URL: http://host.docker.internal:3000/api/ | ||
| UI_BASE_URL: http://host.docker.internal:3000 | ||
| API_PASSWORD: admin | ||
| UI_PASSWORD: admin | ||
| PROJECT_DIR: /workspace |
| compose() { | ||
| SEMAPHORE_IMAGE="$selected_semaphore_image" \ | ||
| docker compose --project-name "$compose_project" --file "$compose_file" "$@" | ||
| docker-compose --project-name "$compose_project" --file "$compose_file" "$@" | ||
| } |
| --ui-user) | ||
| ui_user="$2" | ||
| shift 2 | ||
| ;; | ||
| --ui-password) | ||
| ui_password="$2" | ||
| shift 2 | ||
| ;; | ||
| *) | ||
| gradle_args+=("$1") | ||
| shift | ||
| ;; |
dantro86
left a comment
There was a problem hiding this comment.
This branch now conflicts with main and overlaps the safe external layer and Allure workflow merged in #4. Please rebase and split the remaining Docker/S3 work into focused PRs. In addition to the existing Copilot findings, the blockers below affect user data safety and the advertised report-publishing result.
| tasks.register<Test>("externalTest") { | ||
| group = "verification" | ||
| description = "Runs Semaphore API tests against a running external instance (use -DSTAND=external)." | ||
| filter { includeTestsMatching("io.bookwright.tests.semaphore.*") } |
There was a problem hiding this comment.
This runs every Semaphore product test against the user-supplied instance. That suite creates projects, access keys, schedules and users, executes/stops tasks, changes RBAC and runners, and includes encryption-rotation scenarios; some state is intentionally persistent. externalSmokeTest above is also mutating. An external command must select only explicitly read-only scenarios. Current main already provides the @External-tagged health/auth/system-info/projects checks, so please retain that boundary when rebasing.
| <div class="container"> | ||
| <h1>📊 Semaphore Test Results</h1> | ||
| <p>Automated test results uploaded to S3</p> | ||
| <div id="reports" class="report-list"></div> |
There was a problem hiding this comment.
The generated root index never receives any report entries: #reports remains empty, there is no script/data population, and every publish overwrites index.html with the same No published results yet page. The timestamped report exists only if someone knows its direct key, so the advertised report index is non-functional. Please generate at least the current report link (and preserve/update prior entries if history is intended).
| upload_to_s3() { | ||
| local file_path="$1" | ||
| local s3_key="$2" | ||
| local content_type="${3:-text/html}" |
There was a problem hiding this comment.
content_type is accepted but never used by either upload path. In the raw AWS PUT path the HTML objects therefore have no Content-Type: text/html, so S3 can serve/download them as generic binary data instead of a viewable report. Add the header (and include it consistently in the SigV4 canonical/signed headers) or use a maintained S3 client that sets object metadata.
No description provided.