Skip to content

test(settings): default updates.tier to "off" so version-badge can't block chat-icon clicks#7800

Closed
JohnMcLear wants to merge 2 commits into
developfrom
chore/test-settings-disable-updater
Closed

test(settings): default updates.tier to "off" so version-badge can't block chat-icon clicks#7800
JohnMcLear wants to merge 2 commits into
developfrom
chore/test-settings-disable-updater

Conversation

@JohnMcLear
Copy link
Copy Markdown
Member

Why

v2.7.x added a "severely outdated" version-badge that paints absolutely-positioned at bottom: 8px; right: 8px; z-index: 9999 when the running core is at least one major behind the latest GitHub release. #chaticon sits in the same corner (bottom: 0; right: 25-30px; z-index: 400). The banner intercepts pointer events on the chat icon — every Playwright spec that clicks it times out.

This bit ep_comments_page on 2026-05-17 the moment v3.0.0/v3.1.0 were tagged: their Node-22 matrix pins core to v2.7.3 (engineStrict compatibility) and v2.7.3 lets updates.tier default to "notify", so the updater polls GitHub, finds 3.x, paints the banner, every chat-related test → 1.5min timeout × 3 retries × N tests. Fixed in ether/ep_comments_page#421 with a per-workflow injection.

What

Set updates.tier: "off" in src/tests/settings.json. With tier=off, expressCreateServer in node/hooks/express/updateStatus.ts:65 short-circuits and never registers /api/version-status — the client fetch 404s and the badge stays hidden.

Side effects

  • Plugins that do the standard cp src/tests/settings.json settings.json against any future tag built from this commit get the fix automatically.
  • Plugins pinned to v2.7.3 or earlier still need the per-workflow injection pattern (their checked-out test settings predate this commit).
  • Existing pad-version-badge.spec.ts mocks the endpoint browser-side via page.route() — independent of the server-side setting, keeps passing.

Test plan

  • CI green (backend + frontend + admin)
  • pad-version-badge.spec.ts continues to pass (specs mock the endpoint browser-side)

🤖 Generated with Claude Code

The pad-side version-badge polls `/api/version-status` on every pad load
and paints an absolutely-positioned banner at bottom-right (z-index 9999)
when the running core is at least one major behind the latest GitHub
release. The banner intercepts pointer events on `#chaticon`, which
sits in the same corner at z-index 400 — every Playwright spec that
clicks the chat icon then times out.

This bit ep_comments_page on 2026-05-17 (PR ether/ep_comments_page#421):
their Node-22 matrix pins core to v2.7.3 for engineStrict compatibility,
and the day v3.0.0/v3.1.0 dropped on GitHub the matrix turned
permanently red because the v2.7.3 test settings let `updates.tier`
default to "notify".

Set `updates.tier: "off"` in the standard test settings so:

  - `expressCreateServer` in `node/hooks/express/updateStatus.ts:65`
    short-circuits and never registers `/api/version-status`
  - the client fetch 404s, badge stays hidden
  - any plugin that does `cp src/tests/settings.json settings.json`
    against a recent core gets the fix for free

The existing pad-version-badge specs mock the endpoint browser-side via
`page.route()` (see src/tests/frontend-new/specs/pad-version-badge.spec.ts),
so they're independent of the server-side setting and keep passing.

Doesn't help plugins pinned to v2.7.3 or earlier (their checked-out
test settings predate this commit) — those need the per-workflow
injection pattern from ep_comments_page#421. Going forward, plugins
pinning to any tag built from this commit onward are protected.

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

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

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

Review Summary by Qodo

Disable updater in test settings to prevent version-badge blocking

🧪 Tests

Grey Divider

Walkthroughs

Description
• Disable version-badge updater in test settings
• Prevents z-index collision blocking chat-icon clicks
• Sets updates.tier to "off" in test configuration
• Protects plugins copying standard test settings
Diagram
flowchart LR
  A["Test Settings<br/>updates.tier off"] -- "Disables" --> B["Version Status API<br/>Registration"]
  B -- "Prevents" --> C["Version Badge<br/>Rendering"]
  C -- "Unblocks" --> D["Chat Icon<br/>Clicks"]
Loading

Grey Divider

File Changes

1. src/tests/settings.json 🧪 Tests +1/-1

Add updates.tier off to test settings

• Added "updates":{"tier":"off"} configuration object
• Prevents /api/version-status endpoint registration
• Disables version-badge rendering during tests
• Eliminates z-index collision with chat icon

src/tests/settings.json


Grey Divider

Qodo Logo

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

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

Code Review by Qodo

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

Context used

Grey Divider


Remediation recommended

1. Updater fix lacks regression test 📘 Rule violation ☼ Reliability
Description
This PR changes test runtime behavior by setting updates.tier to off, but the PR diff does not
include any regression test to ensure the version-badge cannot block chat-icon clicks if the setting
is reverted. Without a dedicated test, the issue can silently return and reintroduce CI timeouts.
Code

src/tests/settings.json[1]

+{"title":"Etherpad","favicon":null,"skinName":"colibris","skinVariants":"super-light-toolbar super-light-editor light-background","ip":"0.0.0.0","port":9001,"showSettingsInAdminPage":true,"dbType":"dirty","dbSettings":{"filename":"var/dirty.db"},"defaultPadText":"Welcome to Etherpad!\n\nThis pad text is synchronized as you type, so that everyone viewing this page sees the same text. This allows you to collaborate seamlessly on documents!\n\nGet involved with Etherpad at https://etherpad.org\n","padOptions":{"noColors":false,"showControls":true,"showChat":true,"showLineNumbers":true,"useMonospaceFont":false,"userName":null,"userColor":null,"rtl":false,"alwaysShowChat":false,"chatAndUsers":false,"lang":null},"padShortcutEnabled":{"altF9":true,"altC":true,"cmdShift2":true,"delete":true,"return":true,"esc":true,"cmdS":true,"tab":true,"cmdZ":true,"cmdY":true,"cmdI":true,"cmdB":true,"cmdU":true,"cmd5":true,"cmdShiftL":true,"cmdShiftN":true,"cmdShift1":true,"cmdShiftC":true,"cmdH":true,"ctrlHome":true,"pageUp":true,"pageDown":true},"suppressErrorsInPadText":false,"requireSession":false,"editOnly":false,"minify":true,"maxAge":21600,"soffice":null,"allowUnknownFileEnds":true,"requireAuthentication":false,"requireAuthorization":false,"trustProxy":false,"cookie":{"keyRotationInterval":86400000,"sameSite":"Lax","sessionLifetime":864000000,"sessionRefreshInterval":86400000},"disableIPlogging":false,"automaticReconnectionTimeout":0,"scrollWhenFocusLineIsOutOfViewport":{"percentage":{"editionAboveViewport":0,"editionBelowViewport":0},"duration":0,"scrollWhenCaretIsInTheLastLineOfViewport":false,"percentageToScrollWhenUserPressesArrowUp":0},"users":{"admin":{"password":"changeme1","is_admin":true},"user":{"password":"changeme1","is_admin":false}},"socketTransportProtocols":["websocket","polling"],"socketIo":{"maxHttpBufferSize":1000000},"loadTest":false,"dumpOnUncleanExit":false,"importExportRateLimiting":{"windowMs":90000,"max":10},"importMaxFileSize":52428800,"commitRateLimiting":{"duration":1,"points":10},"exposeVersion":false,"loglevel":"INFO","customLocaleStrings":{},"enableAdminUITests":true,"enablePadWideSettings":true,"lowerCasePadIds":false,"updates":{"tier":"off"},"sso":{"issuer":"${SSO_ISSUER:http://localhost:9001}","clients":[{"client_id":"${ADMIN_CLIENT:admin_client}","client_secret":"${ADMIN_SECRET:admin}","grant_types":["authorization_code"],"response_types":["code"],"redirect_uris":["${ADMIN_REDIRECT:http://localhost:9001/admin/}","https://oauth.pstmn.io/v1/callback"]},{"client_id":"${USER_CLIENT:user_client}","client_secret":"${USER_SECRET:user}","grant_types":["authorization_code"],"response_types":["code"],"redirect_uris":["${USER_REDIRECT:http://localhost:9001/}"]}]}}
Evidence
PR Compliance ID 2 requires that bug fixes include a regression test. The diff shows a behavioral
change in test settings (updates.tier set to off) but no accompanying test addition/modification
in the provided PR changes to guard against regression.

src/tests/settings.json[1-1]
Best Practice: Repository guidelines

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR fixes a test-breaking UI overlay by changing `src/tests/settings.json` (`updates.tier` set to `off`), but it does not add a regression test that would fail if this configuration change is reverted.

## Issue Context
A regression test should directly validate the intended outcome (e.g., chat icon remains clickable / version badge does not intercept clicks under the default test settings) so future changes to updater defaults or routing cannot reintroduce CI timeouts unnoticed.

## Fix Focus Areas
- src/tests/settings.json[1-1]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Source-level lint that fails fast if `updates.tier` in
src/tests/settings.json is reverted from "off" back to "notify"
(the implicit default). Addresses Qodo review note on #7800:

  "This PR changes test runtime behavior by setting updates.tier
  to off, but the PR diff does not include any regression test to
  ensure the version-badge cannot block chat-icon clicks if the
  setting is reverted."

The actual functional regression (badge blocking #chaticon) is
already covered by every chat.spec.ts click — they'd time out
again under "notify" once any new major drops on GitHub. This
new pin-test catches the regression at lint time instead, so we
don't need a downstream npm release to trigger it.

Same pattern as backend-tests-flake-mitigation.test.ts: cheap
source-level lint guarding a behavioural invariant that lives in
config.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@JohnMcLear
Copy link
Copy Markdown
Member Author

Closing this in favor of another approach #7799

@JohnMcLear JohnMcLear closed this May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant