Skip to content

test: fix three failures on main from #1455 and #1459 behaviour changes - #1464

Merged
dviejokfs merged 3 commits into
mainfrom
fix/main-ci-test-failures
Oct 13, 2026
Merged

dviejokfs merged 3 commits into
mainfrom
fix/main-ci-test-failures

Conversation

@dviejokfs

Copy link
Copy Markdown
Contributor

Summary

Three jobs were red on main in the "Rust Tests" workflow (run 38302551181, head 117ecd4). Each one traces to a deliberate behaviour change that merged without its tests being updated. In all three the product behaviour is right and the test (or scenario client) encoded the old behaviour, so this PR changes tests/scenarios only — no product code, no weakened assertions, no skips or retries.

# Failing job Introduced by Kind
1 Integration Tests (docker-providers) #1455 test encoded wrong expectation
2 E2E Deployment Tests (deployments-layout) #1459 test encoded wrong expectation
3 Scenario E2E / Scenarios (recovery) #1455 client did not send newly required consent

None of these is a flake: all three are deterministic. (The deployments-layout failure had also been red on the earlier main runs for ccda4ea99 and a883ff161.)

1. concurrent_duplicate_service_names_preserve_original_database

Root cause. #1455 added ensure_rename_keeps_data to update_service: a rename of a local service that already owns Docker resources is refused with DataSafetyRefusal before its container is stopped, because a volume cannot follow a rename. The test renamed the running original service onto an occupied destination and expected ServiceResourceConflict, which the new guard now preempts (panic at the matches!(conflict, ServiceResourceConflict) assertion).

Fix. The test now asserts both behaviours:

  • renaming the original service (which owns resources) is refused with DataSafetyRefusal naming that service;
  • the occupied-destination check (ensure_new_local_service_resources_unused) is reached from a stopped service row that owns no Docker resources, where it is still the check that refuses, with ServiceResourceConflict.

The existing assertions that the original container, its data (name_guard_marker) and the occupied container are untouched are kept.

Verification. Docker available locally (Colima). On the unmodified test: FAILED with the identical panic at services.rs:18155. With the fix: cargo test --lib -p temps-providers --features docker-tests concurrent_duplicate_service_names_preserve_original_database passes (~8–10 s, real containers). rustfmt --check clean, cargo check --lib -p temps-providers no warnings.

2. deployments-layout specs (content separate at 1920px … 320px)

Root cause. #1459 made canRollbackTo require !is_current (rolling back to the deployment that is already live would only redeploy it), with unit tests in deployment-rollback.test.ts. The spec opened the menu of #3733, which its fixture marks is_current: true, and expected Rollback to this. Failed on every width and on retry because it never depended on timing.

Fix. The spec now asserts that the current deployment offers Redeploy but no rollback entry, then opens the menu of #3732 (status stopped, not current) and asserts Rollback to this is visible. That is the case #1459 set out to enable, so the spec now covers it end to end.

Verification. Built the console bundle and served it locally with the API mocked as the spec does. The previous spec fails against it with the same getByRole('menuitem', { name: 'Rollback to this' }) timeout as CI; the updated spec passes at all six widths (bunx playwright test e2e/anonymous/deployments-layout.spec.ts --project=chromium-anonymous).

3. backup-restore-scenario (Scenarios / recovery)

Root cause. #1455 makes POST /backups/external-services/{id}/run answer 409 RESTART_CONFIRMATION_REQUIRED when the backup would restart a PostgreSQL service whose WAL archiving is off, unless the body carries confirm_restart: true (the console and CLI ask the operator first). The scenario provisions exactly such a fresh service and sent no confirmation, so it failed at trigger a real backup. The shard stops at the first failing scenario, so pitr-scenario (same call, same service shape) would have failed next and is fixed here too. The other restore scenarios use engines for which backup_restart_reason is always None, and pg-upgrade-scenario takes its backup internally, so neither is affected.

Fix. apps/temps-e2e/src/lib/backup-run.ts adds a typed confirmedFullBackup() helper and both scenarios use it. The committed @temps-sdk/api request type predates confirm_restart (the package is not regenerated with each API change), so the helper extends the generated type rather than hand-editing generated code. A full regeneration of packages/api is a separate, much larger change. README step updated.

Verification. I did not run the full scenario shard locally (it needs the release binary, MinIO and a registry). Verified: the 409 body in the CI log matches the handler (run_external_service_backup) and confirm_restart is an Option<bool> read there; the helper typechecks against the committed @temps-sdk/api types (and a negative case fails as expected). The scenarios themselves are exercised by this PR's CI.

Notes

David Viejo added 3 commits October 13, 2026 00:41
…ce that owns Docker resources

#1455 made update_service refuse a rename of any local service that already
owns Docker resources under its current name, before its container is
stopped, because a volume cannot follow a rename. The rename-conflict test
still renamed the running original service onto an occupied destination and
expected ServiceResourceConflict, which that guard now preempts.

Assert the guard itself on the original service, and reach the
destination-occupied check from a stopped service that owns no resources,
where it is still the check that refuses. The original container, its data
and the occupied container are asserted untouched in both cases.

Signed-off-by: David Viejo <dviejokfs@temps.sh>
…ackup

#1455 refuses POST /backups/external-services/{id}/run with 409
RESTART_CONFIRMATION_REQUIRED when the backup would restart a PostgreSQL
service whose WAL archiving is off, unless the request carries
confirm_restart=true. The backup-restore and pitr scenarios provision such a
service and sent no confirmation, so the recovery shard failed at the backup
step.

Send confirm_restart=true from both scenarios. The committed @temps-sdk/api
request type predates the field, so a small typed helper extends the
generated type instead of editing generated code.

Signed-off-by: David Viejo <dviejokfs@temps.sh>
#1459 stopped offering 'Rollback to this' for the deployment that is already
serving traffic (canRollbackTo requires !is_current), which is covered by
unit tests. The deployments-layout spec still opened the menu of the current
deployment and expected the entry.

Assert that the current deployment offers Redeploy but no rollback, and that
a stopped, non-current deployment does offer it.

Signed-off-by: David Viejo <dviejokfs@temps.sh>
@github-actions

Copy link
Copy Markdown

📓 Changelog preview

This is what your commits will add to the generated CHANGELOG.md at release time (via git-cliff). Do not edit CHANGELOG.md by hand — it is generated from your Conventional Commit messages.

## [Unreleased]

### Testing

- **providers:** Expect the data-safety refusal when renaming a service that owns Docker resources
- **e2e:** Confirm the database restart when taking the first WAL-G backup
- **web:** Assert rollback on a stopped deployment, not the current one

@greptile-apps

greptile-apps Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium impact] This PR appears safe to merge.

Summary

This PR updates tests and scenario requests to match the existing rename, rollback, and backup behavior. It does not change product code.

  • The rename test checks data-safety and destination conflicts separately.
  • Deployment menu checks now match current and superseded releases.
  • The backup scenarios send consent before starting a full backup.

Acknowledgment: dviejokfs explicitly deferred full regeneration of packages/api as a separate, larger change. The local request type keeps the generated fields and adds restart consent.

Reviews (1) · Last reviewed commit: "test(web): assert rollback on a stopped ..." · Reviewed by Greptile

@dviejokfs
dviejokfs merged commit 8fd2836 into main Oct 13, 2026
35 of 37 checks passed
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