Skip to content

fix(tests): unblock CI — Windows updater paths + admin-plugins row count#7712

Merged
JohnMcLear merged 2 commits intodevelopfrom
fix/updater-windows-paths
May 10, 2026
Merged

fix(tests): unblock CI — Windows updater paths + admin-plugins row count#7712
JohnMcLear merged 2 commits intodevelopfrom
fix/updater-windows-paths

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Summary

  • The Windows backend test job has been red on develop since Clean update path #7607 merged (run 25623510470). Two new vitest specs assert hardcoded POSIX paths against copyFile calls, but the production code builds the same paths via path.join — which emits backslashes on Windows. So the tests pass on Linux/macOS only by coincidence.
  • Switches the expected values in RollbackHandler.test.ts and UpdateExecutor.test.ts to path.join(deps.repoDir, …) / path.join(deps.backupDir, …), so the assertions track whatever the implementation produces on the host platform.
  • No production code changes; this is test-only and unblocks every PR currently red against develop's Windows backend job (e.g. feat(pad): scrub history in-place on the pad URL (#7659) #7710).

Test plan

  • pnpm exec vitest run tests/backend-new/specs/updater/RollbackHandler.test.ts tests/backend-new/specs/updater/UpdateExecutor.test.ts passes locally on Linux (17/17).
  • Windows backend tests pass on CI for this branch.
  • After merge, the same job on develop @ HEAD goes green.

🤖 Generated with Claude Code

The Windows backend job has been red on develop since #7607 (tier-2
auto-update) merged: RollbackHandler.test.ts:122 and
UpdateExecutor.test.ts:66 asserted POSIX-style paths, but the
implementations build the same paths via path.join — which emits
backslashes on Windows. The tests pass on Linux/macOS only by
coincidence.

Switch the expected values to path.join(deps.repoDir, ...) /
path.join(deps.backupDir, ...) so they track whatever the
implementation produces on the host platform.

No production code changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@qodo-code-review
Copy link
Copy Markdown

ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

Review Summary by Qodo

Fix updater tests to use path.join for cross-platform paths

🐞 Bug fix 🧪 Tests

Grey Divider

Walkthroughs

Description
• Fix Windows path handling in updater tests using path.join
• Replace hardcoded POSIX paths with platform-aware path construction
• Unblock Windows backend CI job failures on develop branch
• No production code changes, test-only fix
Diagram
flowchart LR
  A["Hardcoded POSIX paths<br/>in test assertions"] -- "Replace with path.join" --> B["Platform-aware paths<br/>via path.join"]
  B -- "Matches implementation<br/>on all platforms"] --> C["Tests pass on<br/>Windows and Linux"]
Loading

Grey Divider

File Changes

1. src/tests/backend-new/specs/updater/RollbackHandler.test.ts 🐞 Bug fix +3/-2

Use path.join for cross-platform path assertions

• Import path module from node:path
• Replace hardcoded /srv/etherpad/var/update-backup/pnpm-lock.yaml with `path.join(deps.backupDir,
 'pnpm-lock.yaml')`
• Replace hardcoded /srv/etherpad/pnpm-lock.yaml with path.join(deps.repoDir, 'pnpm-lock.yaml')
• Ensures test assertions match implementation's path construction on all platforms

src/tests/backend-new/specs/updater/RollbackHandler.test.ts


2. src/tests/backend-new/specs/updater/UpdateExecutor.test.ts 🐞 Bug fix +5/-1

Use path.join for cross-platform path assertions

• Import path module from node:path
• Replace hardcoded /srv/etherpad/pnpm-lock.yaml with path.join(deps.repoDir, 'pnpm-lock.yaml')
• Replace hardcoded /srv/etherpad/var/update-backup/pnpm-lock.yaml with `path.join(deps.backupDir,
 'pnpm-lock.yaml')`
• Reformatted object structure for improved readability

src/tests/backend-new/specs/updater/UpdateExecutor.test.ts


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown

qodo-free-for-open-source-projects Bot commented May 10, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

Qodo Logo

ep_set_title_on_pad@0.7.2 took on ep_plugin_helpers as a transitive
plugin dependency, so installing it now adds two rows to the
installed-plugins table (the new plugin + its helper plugin) rather
than one. The Playwright spec hard-coded `toHaveCount(2)` after install
and `toHaveCount(1)` after uninstall, so it has been red on develop
since #7705 merged the new admin bundle (every Frontend admin tests
job, every Node version).

Switch the assertions to scope by row text — `tr` containing
`ep_set_title_on_pad` — and check that exactly one such row exists
after install and zero after uninstall. This survives whatever
transitive plugin deps the chosen test plugin pulls along, which is
the only thing this spec actually cares about.

Verified locally on a fresh Etherpad install: 3/3 admin-update-plugins
tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JohnMcLear JohnMcLear changed the title fix(updater): build expected paths via path.join in updater tests fix(tests): unblock CI — Windows updater paths + admin-plugins row count May 10, 2026
@JohnMcLear JohnMcLear merged commit 2adc228 into develop May 10, 2026
43 checks passed
@JohnMcLear JohnMcLear deleted the fix/updater-windows-paths branch May 10, 2026 11:23
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