Skip to content

Stabilize e2e logout propagation test#37403

Merged
silverwind merged 6 commits intogo-gitea:mainfrom
silverwind:fix-e2e-logout-flake
Apr 24, 2026
Merged

Stabilize e2e logout propagation test#37403
silverwind merged 6 commits intogo-gitea:mainfrom
silverwind:fix-e2e-logout-flake

Conversation

@silverwind
Copy link
Copy Markdown
Member

@silverwind silverwind commented Apr 24, 2026 •
edited
Loading

The events › logout propagation e2e test (example flake) was racing the SSE connection setup: if page2's SharedWorker had not finished registering its messenger by the time page1 triggered logout, the event was silently dropped and page2 stayed on the authenticated page.

Wait 500ms after verifying page2 is signed in, before triggering the logout from page1, so the SharedWorker has time to register. Comment points at a cleaner future fix (expose a ready attribute on the page) that will also work for the planned WebSocket SharedWorker.

This PR was written with the help of Claude Opus 4.7

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 24, 2026
@silverwind silverwind added type/testing skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Apr 24, 2026
@silverwind silverwind mentioned this pull request Apr 24, 2026
Closed
wxiaoguang
wxiaoguang reviewed Apr 24, 2026
View reviewed changes
Comment thread tests/e2e/events.test.ts Outdated
@silverwind @claude
Retry logout event delivery briefly for racing client connections
72f3a0d 
When a user has multiple tabs open and one triggers logout while another
is still establishing its SSE connection, the previous SendMessageBlocking
would silently drop the event because no messenger was registered yet.

Add SendMessageBlockingWithRetry which polls up to a deadline for a
messenger to appear, and use it from SignOut with a 500ms budget.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@silverwind silverwind force-pushed the fix-e2e-logout-flake branch from 547aae7 to 72f3a0d Compare April 24, 2026 10:09
@silverwind silverwind changed the title Fix flaky e2e logout propagation test Fix logout event race for concurrent tab connections Apr 24, 2026
wxiaoguang
wxiaoguang requested changes Apr 24, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Sorry, I don't think such backend retry is right. The data-race can happen anywhere.

If you think it is a data-race and it needs a "delay", you can add the delay in the frontend's test.

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 24, 2026
@wxiaoguang wxiaoguang marked this pull request as draft April 24, 2026 10:17
@silverwind
Copy link
Copy Markdown
Member Author

silverwind commented Apr 24, 2026 •
edited
Loading

I guess I'm fine to add a hack in the test too. EventSource should be replaced with websocket soon so we don't have to carry such a hack for too long.

silverwind and others added 2 commits April 24, 2026 12:40
@silverwind
Revert "Retry logout event delivery briefly for racing client connect…
6cee3bd 
…ions"

This reverts commit 72f3a0d.
@silverwind @claude
Delay logout trigger to avoid racing SSE connection in e2e test
3f7c821 
Give page2's SharedWorker time to register its SSE connection on the
server before page1 triggers logout — otherwise the event can be
silently dropped when no messenger is registered yet for the user.

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

silverwind commented Apr 24, 2026

It's back to test timeout.

wxiaoguang
wxiaoguang reviewed Apr 24, 2026
View reviewed changes
Comment thread tests/e2e/events.test.ts
@wxiaoguang
Apply suggestion from @wxiaoguang
015219f 
Signed-off-by: wxiaoguang <wxiaoguang@gmail.com>
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Apr 24, 2026
@silverwind silverwind changed the title Fix logout event race for concurrent tab connections Stabilise e2e logout propagation test Apr 24, 2026
@silverwind silverwind marked this pull request as ready for review April 24, 2026 11:00
@silverwind silverwind mentioned this pull request Apr 24, 2026
Merged
@silverwind silverwind changed the title Stabilise e2e logout propagation test Stabilize e2e logout propagation test Apr 24, 2026
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 24, 2026
@silverwind silverwind enabled auto-merge (squash) April 24, 2026 15:32
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 24, 2026
@silverwind silverwind merged commit 0277e3e into go-gitea:main Apr 24, 2026
26 checks passed
@silverwind silverwind deleted the fix-e2e-logout-flake branch April 24, 2026 15:37
@GiteaBot GiteaBot added this to the 1.27.0 milestone Apr 24, 2026
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 24, 2026
@silverwind @claude
Stabilize e2e logout propagation test
f00c016 
Backport of go-gitea#37403 to release/v1.26.

Give page2's SharedWorker time to register its SSE connection on the
server before page1 triggers logout — otherwise the event can be
silently dropped when no messenger is registered yet for the user.

Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 24, 2026
silverwind added a commit that referenced this pull request Apr 24, 2026
@silverwind @claude
Stabilize e2e logout propagation test (#37408)
64d1202 
Backport of #37403 to `release/v1.26`.

The `events › logout propagation` e2e test was racing the SSE connection
setup: if page2's SharedWorker had not finished registering its
messenger by the time page1 triggered logout, the event was silently
dropped and page2 stayed on the authenticated page.

Wait 500ms after verifying page2 is signed in, before triggering the
logout from page1, so the SharedWorker has time to register. Comment
points at a cleaner future fix (expose a ready attribute on the page)
that will also work for the planned WebSocket SharedWorker.

---
This PR was written with the help of Claude Opus 4.7

Co-authored-by: Claude (Opus 4.7) <noreply@anthropic.com>
silverwind added a commit to mohammad-rj/gitea that referenced this pull request Apr 24, 2026
@silverwind
Merge remote-tracking branch 'origin/main' into pr-36965-ci
696e792 
* origin/main:
  Stabilize e2e logout propagation test (go-gitea#37403)
  refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405)
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)

# Conflicts:
#	tests/e2e/events.test.ts
silverwind added a commit to silverwind/gitea that referenced this pull request Apr 25, 2026
@silverwind
Merge remote-tracking branch 'origin/main' into reply
aec2f56 
* origin/main: (51 commits)
  Fix color regressions, add `priority` color (go-gitea#37417)
  [skip ci] Updated translations via Crowdin
  Stabilize e2e logout propagation test (go-gitea#37403)
  refactor: serve site manifest via `/assets/site-manifest.json` endpoint (go-gitea#37405)
  feat(security): set X-Content-Type-Options: nosniff by default (go-gitea#37354)
  Refactor pull request view (1) (go-gitea#37380)
  Improve AGENTS.md (go-gitea#37382)
  Remove dead CSS (go-gitea#37376)
  Add pr-review e2e test and speed up e2e tests (go-gitea#37345)
  Drop Fomantic tab, checkbox and form patches (go-gitea#37377)
  fix: dump with default zip type produces uncompressed zip (go-gitea#37401)
  Allow fast-forward-only merge when signed commits are required (go-gitea#37335)
  Introduce `ActionRunAttempt` to represent each execution of a run (go-gitea#37119)
  Move review request functions to a standalone file (go-gitea#37358)
  Fix repo init README EOL (go-gitea#37388)
  Fix org team assignee/reviewer lookups for team member permissions (go-gitea#37365)
  Remove external service dependencies in migration tests (go-gitea#36866)
  Extend issue context popup beyond markdown content (go-gitea#36908)
  fix: commit status reporting (go-gitea#37372)
  Support for Custom URI Schemes in OAuth2 Redirect URIs (go-gitea#37356)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wxiaoguang wxiaoguang wxiaoguang approved these changes

@bircni bircni bircni approved these changes

Assignees

No one assigned

Labels

lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. type/testing

Projects

None yet

Milestone

1.27.0

Development

Successfully merging this pull request may close these issues.

4 participants

@silverwind @wxiaoguang @bircni @GiteaBot