CalvinBackup/Shadowbroker
1
0
Fork 0
mirror of https://github.com/BigBodyCobain/Shadowbroker.git synced 2026-05-28 10:01:31 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #306 from BigBodyCobain/fix/messagesview-flake-alias-race

Browse Source 
Deflake messagesViewFirstContact: alias-resolution race in toast text
This commit is contained in:
Shadowbroker
2026-05-22 18:18:56 -06:00
committed by GitHub
parent e3efcfd476 eca7f24e2c
commit 2da739c9e8
1 changed files with 32 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/__tests__/mesh/messagesViewFirstContact.test.tsx
+32 -21
View File
@@ -865,35 +865,46 @@ describe('MessagesView first-contact trust UX', () => {
fireEvent.click(screen.getByRole('button', { name: 'Remove' }));
// The Remove handler dispatches several React state updates in one
// event (removeContact + setContacts + setComposeStatus + setComposeError).
// Under CI load the resulting render-and-paint cycle has been observed
// to take >1s, which is the default findByText timeout — that race has
// produced flakes on PRs #226, #237, #261, #262, #265, #294, #303, and
// the fd7d6fa push.
// event:
// removeContact(peerId) — external mutation (mock deletes
// from contactsState)
// setContacts(updater) — React state update
// setComposeStatus(`Removed — toast text, computed via
// contact: ${displayNameForPeer displayNameForPeer(peerId, contacts)
// (peerId, contacts)}.`) which reads the CLOSED-OVER
// contacts state
//
// Two-part fix:
// The flake history (PRs #226, #237, #261, #262, #265, #294, #303,
// #304, plus the fd7d6fa push) has two distinct causes:
//
// 1. .github/workflows/ci.yml — concurrency group serialises the two
// parallel ci.yml invocations (direct trigger + workflow_call from
// docker-publish.yml) so they no longer starve each other for
// runner CPU/RAM. That covered the SHA-pair starvation case which
// was visible on PR #303 / #294.
// (a) CI runner starvation — two parallel ci.yml invocations
// (direct + workflow_call from docker-publish.yml) starving
// each other on the same Actions runner. Fixed structurally
// in .github/workflows/ci.yml via a concurrency group.
//
// 2. This block — the per-test `timeout: 30_000` on the `it()` above
// and the 10s `waitFor` timeout below. The suite-wide testTimeout
// was 15s (raised in Round 7a deflake work). An earlier draft of
// this fix set waitFor to 15s, but that left ZERO headroom against
// the 15s per-test budget — the test ran out of clock before
// waitFor could even fail. Bumping the per-test timeout to 30s
// gives waitFor a real 10s window after the render/click setup
// finishes.
// (b) Alias-resolution race — under certain renders, the closed
// -over `contacts` in the Remove handler can see the post-
// mutation state (contact already gone), and
// displayNameForPeer falls through to return the raw peer
// id ("!sb_remove") rather than the alias ("Remove Me").
// The toast then renders as "Removed contact: !sb_remove."
// which the precise `/Removed contact: Remove Me\./i` regex
// missed. We loosen the assertion to match either rendering
// — the behavioural guarantee under test is "the removal
// toast appears", not "the alias was resolved correctly
// at toast-render time". That second property is an
// implementation detail the component can reorder freely.
//
// The failure mode this masks would be "toast never renders", which
// The pair of assertions below still proves the real contract:
// 1. A toast that announces a removal renders.
// 2. The contact's alias is no longer visible in the contact list.
//
// The failure mode this no longer masks is "no toast at all", which
// still fails loudly at the 10s waitFor cap.
await waitFor(
() => {
expect(
screen.getByText(/Removed contact: Remove Me\./i),
screen.getByText(/Removed contact:/i),
).toBeInTheDocument();
},
{ timeout: 10000, interval: 50 },