Skip to content

test(nextjs): Add webpack variant for span-streaming e2e app - #22704

Merged
chargome merged 1 commit into
developfrom
test/nextjs-streaming-webpack-variant
Jul 27, 2026
Merged

test(nextjs): Add webpack variant for span-streaming e2e app#22704
chargome merged 1 commit into
developfrom
test/nextjs-streaming-webpack-variant

Conversation

@chargome

@chargome chargome commented Jul 27, 2026

Copy link
Copy Markdown
Member

Discovered in #22674 that this was not running on webpack so here we go.

I added a reusable helper to aggregate streamed spans for e2e tests

Adds a webpack build variant to the nextjs-16-streaming e2e app so the
webpack + span-streaming combination has coverage. Adds a reusable
`collectStreamedSpans` test-util that accumulates streamed spans across
envelopes, and uses it in the server-component tests (streamed child spans
can arrive in a different envelope than their segment root).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chargome chargome self-assigned this Jul 27, 2026
@chargome
chargome marked this pull request as ready for review July 27, 2026 13:45
@chargome
chargome requested review from Lms24 and andreiborza July 27, 2026 13:45
@Lms24

Lms24 commented Jul 27, 2026

Copy link
Copy Markdown
Member

good catch, thx!

}

test('Sends a streamed span for a request to app router with URL', async ({ page }) => {
test.skip(isDevMode, 'Turbopack intermittently returns 404 for nested dynamic routes in dev mode');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The isDevMode check is too broad, causing tests to be skipped for the new development-webpack environment, which prevents the intended test coverage from being added.
Severity: MEDIUM

Suggested Fix

Create a more specific isTurbopackDevMode constant in nextjs-16-streaming/utils/isDevMode.ts that uses strict equality: process.env.TEST_ENV === 'development'. Update the test.skip() calls in server-components.test.ts to use this new constant. This ensures tests are only skipped for Turbopack and not for the new webpack dev environment.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location:
dev-packages/e2e-tests/test-applications/nextjs-16-streaming/tests/server-components.test.ts#L15

Potential issue: The `isDevMode` constant in `nextjs-16-streaming` is defined using
`process.env.TEST_ENV.includes('development')`. The pull request introduces a new test
environment, `development-webpack`, to add test coverage for webpack. However, because
`'development-webpack'` includes the substring `'development'`, the `isDevMode` check
evaluates to `true`. This causes all server-component tests that use
`test.skip(isDevMode, ...)` to be skipped when running in the new webpack dev mode. As a
result, the intended webpack test coverage is not added, leaving these components
untested in that environment.

Also affects:

  • dev-packages/e2e-tests/test-applications/nextjs-16-streaming/tests/server-components.test.ts:32
  • dev-packages/e2e-tests/test-applications/nextjs-16-streaming/tests/server-components.test.ts:53

Did we get this right? 👍 / 👎 to inform future reviews.

@Lms24

Lms24 commented Jul 27, 2026

Copy link
Copy Markdown
Member

i think we should probably use this function whenever we want to assert on more than one span in a trace. Everything else has potential to flake. Will keep this in mind when migrating more tests to streaming and write the migration skill accordingly

@chargome
chargome merged commit cc3fe1f into develop Jul 27, 2026
240 of 241 checks passed
@chargome
chargome deleted the test/nextjs-streaming-webpack-variant branch July 27, 2026 15:39
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.

2 participants