Skip to content

docs(issues): add specification for #2179 - #2194

Open
da2ce7 wants to merge 6 commits into
torrust:developfrom
da2ce7:docker-e2e-job-package-flag-spec
Open

docs(issues): add specification for #2179#2194
da2ce7 wants to merge 6 commits into
torrust:developfrom
da2ce7:docker-e2e-job-package-flag-spec

Conversation

@da2ce7

@da2ce7 da2ce7 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Related to #2179

Summary

This pull request adds the reviewed specification for issue #2179 and contains no implementation, so it does not resolve the issue.

The specification describes a defect in .github/workflows/testing.yaml: the docker-e2e job cannot reach its own E2E tests, because its four runner steps invoke cargo run --bin e2e_tests_runner … and cargo run --bin qbittorrent_e2e_runner … without naming the package that owns those binaries. Both live in torrust-tracker-e2e-tools under packages/e2e-tools/src/bin/, and the workspace root is itself a package with a default-run, so from the root cargo run --bin <name> resolves only against the root package's targets and the step fails before any tracker image is exercised, with error: no bin target named 'e2e_tests_runner' in default-run packages and exit code 101. The same four invocations in .github/workflows/container.yaml already carry -p torrust-tracker-e2e-tools, which is why that workflow is green.

The specification also traces where the two workflows diverged, and it is not the job's if: guard. Commit c1b6c5466 added only the guard and its comment. Commit c47173f53 moved the runner sources into packages/e2e-tools as pure renames and touched neither workflow, breaking both at once. Commit 2d1ce246b then repaired container.yaml alone, adding the package flag and wrapping the long first step in a >- block scalar; the identical four lines in testing.yaml were not part of that change. What the guard did contribute is the silence: docker-e2e is skipped on pushes to develop and on pull requests targeting develop, so upstream CI has never executed the broken steps and develop has stayed green while the job has failed, since June 2026, on every feature-branch push that starts a Testing run at all — the workflow ignores pushes whose changed files are all Markdown or project-words.txt, so a documentation-only push starts no run and produces no failure.

What the fix has to get right

One detail of the repair is load-bearing rather than cosmetic. .yamllint-ci.yml sets a 200-character line-length limit that linter all enforces over the workflow files. The run-tracker-e2e-tests line is 184 characters today, and the flag plus its separating space adds 29, taking it to 213. The other three lines are 153, 151 and 156 characters and land at 182, 180 and 185, inside the limit. The first step therefore has to be wrapped in a >- block scalar, which is exactly the form container.yaml already uses for the same step and for the same reason.

The specification is equally explicit about verification, because the obvious route does not work: a pull request targeting develop cannot prove the fix, since the guard skips the job for precisely that event. Evidence has to come from a feature-branch push where the guard admits the job (scenario M1), with a second scenario confirming that behaviour on develop is unchanged (M2).

Evidence recomputed before this branch was opened

Both evidence claims were re-checked against develop at 89d45145 rather than carried over from the drafting session. The four cargo run invocations still carry no package flag at lines 185, 189, 193 and 197, and still measure 184, 153, 151 and 156 characters, so the block-scalar requirement stands as written. The symptom is live: in the Testing run for 4bff469e on the da2ce7/torrust-tracker fork, the Docker E2E job builds the tracker image successfully and then fails at step Run E2E Tests, leaving the three qBittorrent steps skipped. That run is cited in the specification, replacing the drafting note that the observations had not been reproduced.

Files touched

Only docs/issues/. Against develop the branch adds exactly one file, docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md (the branch history carries it from the draft location it was renamed from), so no source, test, workflow or manifest is touched here. The workflow repair itself follows in a separate implementation pull request.

Validation performed

linter all passes with exit code 0 at the branch tip in 27.6 s, covering markdown, local Markdown links and fragments via lychee, YAML, TOML, cspell, clippy, rustfmt and shellcheck. git diff --check over the branch range against develop reports no whitespace defects.

The moved document contains no relative file links, only two absolute GitHub URLs and no intra-document anchors, so the change of directory cannot break a target; lychee confirms this independently. Every repository path named in inline code was resolved against the tree, and the paths that do not resolve are the pre-move src/bin/ locations the specification describes as history, which is what it intends.

The reviewed draft now has a GitHub issue, so the specification for the broken Docker E2E job moves out of the drafts folder into `docs/issues/open/` under its assigned number, and its frontmatter, heading, workflow checkpoints and progress log record that number together with the spec-only delivery branch.

Both evidence claims were recomputed against `develop` at 89d4514 before the move: the four `cargo run` invocations in the `docker-e2e` job still carry no package flag at lines 185, 189, 193 and 197, measuring 184, 153, 151 and 156 characters, so the block-scalar requirement on the first step stands; and the symptom is now cited from an observed fork run rather than only from reported ones. The out-of-scope note on the stale `src/bin/` inventory names torrust#2190, which has since taken ownership of those references.
Copilot AI lite review requested due to automatic review settings September 10, 2026 08:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Documentation corrections remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds the reviewed specification for issue #2179, documenting the Docker E2E workflow defect and planned fix.

Changes:

  • Documents the Cargo package-resolution issue and workflow divergence.
  • Defines implementation scope, constraints, and verification scenarios.
  • Adds the issue specification and supporting references.
File summaries
File Summary
docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md Adds the complete issue specification and implementation plan.
Review details

Suppressed comments (4)

docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md:168

  • This repeats the same path-filter overstatement: container.yaml does not run on every develop push when the push is documentation-only. Please qualify the CI evidence so the spec does not claim validation on events the workflow intentionally skips.
- No material risk to the change itself. The flag value is not a guess: `container.yaml` runs the same four binaries with the same flag against the same package on every push to `develop`, so the invocation form is already proven in CI.

docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md:34

  • The event model here omits the workflow's paths-ignore: testing.yaml skips push/PR events when every changed path is Markdown or project-words.txt (lines 8-18), so the remaining coverage is not literally every feature-branch push or PR. Please qualify this statement and the matching acceptance/verification wording below as events that actually trigger the workflow.
The `docker-e2e` job in `.github/workflows/testing.yaml` (lines 124-197) exists to give E2E coverage to the events that `.github/workflows/container.yaml` does not cover. Its `if:` guard (lines 133-139) skips the job on pull requests whose base is `develop` or `main`, and on pushes to `refs/heads/develop`, `refs/heads/main`, and `refs/heads/releases/*`, because `container.yaml` builds the same image and runs the same E2E tests for exactly those events. The job's comment (lines 125-129) states this and cites issue #1854. What remains after the guard is precisely the feature-branch case: a push to any in-repo branch that is not `develop`, `main`, or a release branch, and any pull request targeting some other branch.

docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md:150

  • testing.yaml has other jobs (unit and layer-bans) that will still run for an implementation PR, so “no existing job changes status” is broader than this guard can guarantee. Narrow M2 to docker-e2e remaining skipped and container.yaml covering the event.
| M2  | Behavior on `develop` is unchanged                 | Open the pull request for this fix against `develop` and inspect its `Testing` workflow run.                                                                  | The `Docker E2E` job is skipped, as it is today, because `container.yaml` covers the event. No new job appears and no existing job changes status.                     | TODO   | Run URL of the pull request's `Testing` workflow      |

docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md:186

  • These references are pull requests, not workflow runs, so labeling them “Reported failing runs” is misleading. Name them as the dependency-update pull requests that reported the failures.
- Reported failing runs: #2055, #2106
  • Files reviewed: 1/1 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md Outdated
Comment thread docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md Outdated
Comment thread docs/issues/open/2179-docker-e2e-job-package-flag/ISSUE.md Outdated
…cification

Qualify the "every push" evidence, correct the line-length arithmetic, and
route manual verification through an issue-local evidence file.

Both `testing.yaml` and `container.yaml` carry `paths-ignore` entries for
`**/*.md` and `project-words.txt` on `push` and `pull_request`, so a push
whose changed files are all documentation starts no run at all. The
specification claimed the broken job fails on every feature-branch push, and
that `container.yaml` proves the invocation form on every push to `develop`;
both hold only for pushes carrying a non-documentation change. The title, the
Background paragraph and both Risks entries now say so.

The inserted reference form `-p torrust-tracker-e2e-tools` plus the space
separating it from the following argument is 29 characters, not 26. The four
`cargo run` lines therefore land at 213, 182, 180 and 185, not 210, 179, 177
and 182. Line 185 still exceeds the 200-character limit in `.yamllint-ci.yml`,
so the `>-` block scalar the repair depends on is unchanged; only the
documented numbers were wrong.

The `create-issue` workflow requires manual-verification evidence in an
issue-local `manual-verification-evidence.md` recording the steps, commands,
output and outcome. The specification asked only for a run URL, which
preserves neither what was done nor what was observed once the run's logs
expire. T2, the in-scope bullet, the commit point, the checkpoint, the
acceptance criterion, both manual scenarios and the AC2 evidence row now name
that file, with the run URL carried inside it as the reference to the run.
@da2ce7

da2ce7 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

The three Copilot threads are processed per process-copilot-suggestions: each has a reply with the decision and its evidence, each is resolved individually, and the tracker record is at docs/copilot-pr-reviews/pr-2194-copilot-suggestions.md (commit 1d920d5).

All three were accepted, and all three were recomputed against develop at 89d45145 before being accepted rather than taken on the report's word. The specification changes are in one commit, b13e062:

  • "Every feature-branch push" — action. Both workflows declare paths-ignore for **/*.md and project-words.txt under push and pull_request, so a documentation-only push starts no run and no job fails. The Background paragraph now carries the qualification and its cause, the heading drops the quantifier, and both Risks entries making the same unqualified claim are corrected. container.yaml is additionally branch-restricted to develop, main and releases/**/*, so it never runs on a feature-branch push at all; the specification now says so.
  • Line-length arithmetic — action. The reference form plus its separating space is 29 characters, not 26, so the four cargo run lines land at 213, 182, 180 and 185, not 210, 179, 177 and 182. The current lengths the specification recorded, 184, 153, 151 and 156, were already right. The repair shape is unchanged: 213 still exceeds the 200-character limit in .yamllint-ci.yml and the other three still fall inside it, so the >- block scalar on the first step still stands alone. This description is corrected to match.
  • Manual-verification evidence — action. create-issue requires the evidence in an issue-local manual-verification-evidence.md, and the specification asked only for a run URL. T2, the in-scope verification bullet, the T2 commit point, the workflow checkpoint, the manual-verification acceptance criterion, both manual scenarios and the AC2 evidence row now name that file and the section each scenario writes, with the run URL carried inside it as the reference to the run.

Validation on the build host: git diff --check over the branch range against develop is clean, and linter all exits 0 twice, at b13e062 in 18.2 s and at the tip 1d920d5 in 18.0 s, so each commit is green on its own. Markdown, local links, YAML, TOML, spelling, clippy, formatting and shell checks all pass.

Follow-up outside this pull request: issue #2179 still carries the unqualified title "Docker E2E job in testing.yaml fails on every feature-branch push". The specification heading no longer does, and the issue title needs the same edit to stay in step.

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.

2 participants