Skip to content

fix: harden flaky stories mark-watched integration test #306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
itsmeadi wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: harden flaky stories mark-watched integration test #306

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 6 additions & 14 deletions packages/feeds-client/__integration-tests__/stories.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
createTestClient,
createTestTokenGenerator,
getTestUser,
markWatchedAndSync,
waitForEvent,
} from './utils';

Expand Down Expand Up @@ -72,16 +73,10 @@ describe('Stories Feed', () => {

it(`user 2 marks the story as watched`, async () => {
await user2StoriesFeed.getOrCreate({ watch: true });
user2StoriesFeed.on('feeds.stories_feed.updated', (_) => {});

await Promise.all([
waitForEvent(user2StoriesFeed, 'feeds.stories_feed.updated'),
user2StoriesFeed.markActivity({
mark_watched: [
user2StoriesFeed.state.getLatestValue().aggregated_activities![0]
.activities![0].id,
],
}),
await markWatchedAndSync(user2StoriesFeed, [
user2StoriesFeed.state.getLatestValue().aggregated_activities![0]
.activities![0].id,
]);

expect(
Expand Down Expand Up @@ -129,11 +124,8 @@ describe('Stories Feed', () => {
}
});

await Promise.all([
waitForEvent(feed, 'feeds.stories_feed.updated'),
feed.markActivity({
mark_watched: [feed.state.getLatestValue().activities![0].id],
}),
await markWatchedAndSync(feed, [
feed.state.getLatestValue().activities![0].id,
]);

expect(feed.state.getLatestValue().activities![0].is_watched).toBe(true);
Expand Down
26 changes: 25 additions & 1 deletion packages/feeds-client/__integration-tests__/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ const WAIT_FOR_EVENT_MS = 30_000;
export const waitForEvent = (
client: FeedsClient | Feed,
type: FeedsEvent['type'] | WSEvent['type'],
timeoutMs = WAIT_FOR_EVENT_MS,
) => {
return new Promise((resolve, reject) => {
const listener = (e: FeedsEvent | WSEvent) => {
Expand All @@ -103,9 +104,32 @@ export const waitForEvent = (
// @ts-expect-error client expects WSEvents
client.off(type, listener);
reject(new Error(`Event not received: ${type}`));
}, WAIT_FOR_EVENT_MS);
}, timeoutMs);

// @ts-expect-error client expects WSEvents
client.on(type, listener);
});
};

/**
* Marks activities as watched and prefers the realtime
* `feeds.stories_feed.updated` event. If that event is dropped by the API
* (known intermittent flake), falls back to reloading the feed.
*/
export const markWatchedAndSync = async (
feed: Feed,
activityIds: string[],
eventTimeoutMs = 5_000,
) => {
const eventPromise = waitForEvent(
feed,
'feeds.stories_feed.updated',
eventTimeoutMs,
);
await feed.markActivity({ mark_watched: activityIds });
try {
await eventPromise;
} catch {
await feed.getOrCreate({ watch: true });
}
Comment on lines +124 to +134

@coderabbitai coderabbitai Bot Jul 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Await the API promise and the event waiter together using Promise.all.

As per coding guidelines, integration tests that trigger API calls and wait for real-time events must await both the API promise and the event using Promise.all to prevent unhandled rejections.

In the current implementation, if feed.markActivity throws an error, the function will exit immediately, leaving eventPromise without a catch handler. When it subsequently times out, it will cause an UnhandledPromiseRejection and crash the test suite.

You can fix this by attaching a catch handler to eventPromise and awaiting both promises concurrently.

🛠️ Proposed fix 
-  const eventPromise = waitForEvent(
-    feed,
-    'feeds.stories_feed.updated',
-    eventTimeoutMs,
-  );
-  await feed.markActivity({ mark_watched: activityIds });
-  try {
-    await eventPromise;
-  } catch {
+  const eventPromise = waitForEvent(
+    feed,
+    'feeds.stories_feed.updated',
+    eventTimeoutMs,
+  ).catch(() => null);
+
+  const [, eventResult] = await Promise.all([
+    feed.markActivity({ mark_watched: activityIds }),
+    eventPromise,
+  ]);
+
+  if (eventResult === null) {
     await feed.getOrCreate({ watch: true });
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const eventPromise = waitForEvent(
feed,
'feeds.stories_feed.updated',
eventTimeoutMs,
);
await feed.markActivity({ mark_watched: activityIds });
try {
await eventPromise;
} catch {
await feed.getOrCreate({ watch: true });
}
const eventPromise = waitForEvent(
feed,
'feeds.stories_feed.updated',
eventTimeoutMs,
).catch(() => null);
const [, eventResult] = await Promise.all([
feed.markActivity({ mark_watched: activityIds }),
eventPromise,
]);
if (eventResult === null) {
await feed.getOrCreate({ watch: true });
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/feeds-client/__integration-tests__/utils.ts` around lines 124 - 134,
Update the flow around feed.markActivity and eventPromise to attach the existing
fallback catch handler to the event waiter, then await the API call and handled
event promise concurrently with Promise.all. Preserve the getOrCreate({ watch:
true }) fallback when the event wait fails.

Source: Coding guidelines

};
Loading