Skip to content
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.

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

Report tests to Datadog from Job that executed them #73180

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Nov 25, 2024

Previously, each test run was associated with a separate Job that is responsible for uploading all reports.
This makes it harder to understand under which configuration a specific test failed.
Now we immediately upload test results from the respective job meaning a test run in Datadog can be associated with the actual CI job that ran it and therefore it's config (Turbopack vs Webpack, PPR vs no PPR, React 18 vs 19)

This increases feedback time slightly by up to 1 minute (the slowest single upload time).
I'd say this is an acceptable tradeoff given our current CI times.

This reduces the time until a "Retry failed jobs" is avaiable since we no longer need to pass the test results between jobs.

Before:
CleanShot 2024-11-25 at 17 46 07

-- https://app.datadoghq.com/ci/test/AwAAAZNglKMHojDJ-AAAABhBWk5nbEtNSEFBQzZvbzJnTHZRV24wclgAAAAkMDE5MzYwOWUtMWM5Ni00MGVjLTk2ZTAtN2NkZTY5ZTVjY2Q2AAC6ug?colorBy=service&currentTab=overview&env=ci&eventStack=AwAAAZNglKMHojDJ-AAAABhBWk5nbEtNSEFBQzZvbzJnTHZRV24wclgAAAAkMDE5MzYwOWUtMWM5Ni00MGVjLTk2ZTAtN2NkZTY5ZTVjY2Q2AAC6ug&graphType=flamegraph&index=citest&spanViewType=metadata&testId=AwAAAZNglKMHojDJ-AAAABhBWk5nbEtNSEFBQzZvbzJnTHZRV24wclgAAAAkMDE5MzYwOWUtMWM5Ni00MGVjLTk2ZTAtN2NkZTY5ZTVjY2Q2AAC6ug

After:

--

Copy link
Member Author

eps1lon commented Nov 25, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@eps1lon eps1lon changed the title Report tests to datadog from Job that executed them Report tests to Datadog from Job that executed them Nov 25, 2024
@ijjk
Copy link
Member

ijjk commented Nov 25, 2024

Failing test suites

Commit: 4b1b811

pnpm test-dev-turbo test/development/app-dir/capture-console-error/capture-console-error.test.ts (turbopack)

  • app-dir - capture-console-error > should capture browser console error in render and dedupe if necessary
  • app-dir - capture-console-error > should capture browser console error in render and dedupe when multi same errors logged
  • app-dir - capture-console-error > should capture server replay string error from console error
  • app-dir - capture-console-error > should capture server replay error instance from console error
Expand output

● app-dir - capture-console-error › should capture browser console error in render and dedupe if necessary

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `app-dir - capture-console-error should capture browser console error in render and dedupe if necessary 1`

- Snapshot  - 1
+ Received  + 0

  {
    "count": 2,
    "description": "trigger an console.error in render",
    "source": "app/browser/render/page.js (4:11) @ Page
-   2 |
  > 4 |   console.error('trigger an console.error in render')
      |           ^",
    "stack": [],
    "title": "Console Error",
  }

  48 |
  49 |     if (isTurbopack) {
> 50 |       await expect(browser).toDisplayCollapsedRedbox(`
     |                             ^
  51 |        {
  52 |          "count": 2,
  53 |          "description": "trigger an console.error in render",

  at Object.toDisplayCollapsedRedbox (development/app-dir/capture-console-error/capture-console-error.test.ts:50:29)

● app-dir - capture-console-error › should capture browser console error in render and dedupe when multi same errors logged

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `app-dir - capture-console-error should capture browser console error in render and dedupe when multi same errors logged 1`

- Snapshot  - 1
+ Received  + 0

  {
    "count": 2,
    "description": "trigger an console.error in render",
    "source": "app/browser/render/page.js (4:11) @ Page
-   2 |
  > 4 |   console.error('trigger an console.error in render')
      |           ^",
    "stack": [],
    "title": "Console Error",
  }

  80 |
  81 |     if (isTurbopack) {
> 82 |       await expect(browser).toDisplayCollapsedRedbox(`
     |                             ^
  83 |        {
  84 |          "count": 2,
  85 |          "description": "trigger an console.error in render",

  at Object.toDisplayCollapsedRedbox (development/app-dir/capture-console-error/capture-console-error.test.ts:82:29)

● app-dir - capture-console-error › should capture server replay string error from console error

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `app-dir - capture-console-error should capture server replay string error from console error 1`

- Snapshot  - 1
+ Received  + 0

  {
    "count": 2,
    "description": "ssr console error:client",
    "source": "app/ssr/page.js (4:11) @ Page
-   2 |
  > 4 |   console.error(
      |           ^",
    "stack": [],
    "title": "Console Error",
  }

  112 |
  113 |     if (isTurbopack) {
> 114 |       await expect(browser).toDisplayCollapsedRedbox(`
      |                             ^
  115 |        {
  116 |          "count": 2,
  117 |          "description": "ssr console error:client",

  at Object.toDisplayCollapsedRedbox (development/app-dir/capture-console-error/capture-console-error.test.ts:114:29)

● app-dir - capture-console-error › should capture server replay error instance from console error

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `app-dir - capture-console-error should capture server replay error instance from console error 1`

- Snapshot  - 1
+ Received  + 0

  {
    "count": 2,
    "description": "Error: page error",
    "source": "app/ssr-error-instance/page.js (4:17) @ Page
-   2 |
  > 4 |   console.error(new Error('page error'))
      |                 ^",
    "stack": [],
    "title": "Console Error",
  }

  144 |
  145 |     if (isTurbopack) {
> 146 |       await expect(browser).toDisplayCollapsedRedbox(`
      |                             ^
  147 |        {
  148 |          "count": 2,
  149 |          "description": "Error: page error",

  at Object.toDisplayCollapsedRedbox (development/app-dir/capture-console-error/capture-console-error.test.ts:146:29)

Read more about building and testing Next.js in contributing.md.

__NEXT_EXPERIMENTAL_PPR=true pnpm test-dev test/development/app-dir/dynamic-io-dev-errors/dynamic-io-dev-errors.test.ts (PPR)

  • Dynamic IO Dev Errors > should show a red box error on the SSR render
  • Dynamic IO Dev Errors > should show a red box error on client navigations
  • Dynamic IO Dev Errors > should display error when component accessed data without suspense boundary
  • Dynamic IO Dev Errors > should clear segment errors after correcting them
Expand output

● Dynamic IO Dev Errors › should show a red box error on the SSR render

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `Dynamic IO Dev Errors should show a red box error on the SSR render 1`

- Snapshot  - 0
+ Received  + 1

@@ -4,8 +4,9 @@
    "source": "app/error/page.tsx (2:23) @ random
  > 2 |   const random = Math.random()
      |                       ^",
    "stack": [
      "JSON.parse <anonymous> (0:0)",
+     "<unknown> <anonymous> (0:0)",
    ],
    "title": "Console Error",
  }

  28 |       `)
  29 |     } else {
> 30 |       await expect(browser).toDisplayCollapsedRedbox(`
     |                             ^
  31 |        {
  32 |          "count": 1,
  33 |          "description": "[ Server ] Error: Route "/error" used \`Math.random()\` outside of \`"use cache"\` and without explicitly calling \`await connection()\` beforehand. See more info here: https://nextjs.org/docs/messages/next-prerender-random",

  at Object.toDisplayCollapsedRedbox (development/app-dir/dynamic-io-dev-errors/dynamic-io-dev-errors.test.ts:30:29)

● Dynamic IO Dev Errors › should show a red box error on client navigations

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `Dynamic IO Dev Errors should show a red box error on client navigations 1`

- Snapshot  - 0
+ Received  + 1

@@ -4,8 +4,9 @@
    "source": "app/error/page.tsx (2:23) @ random
  > 2 |   const random = Math.random()
      |                       ^",
    "stack": [
      "JSON.parse <anonymous> (0:0)",
+     "<unknown> <anonymous> (0:0)",
    ],
    "title": "Console Error",
  }

  66 |       `)
  67 |     } else {
> 68 |       await expect(browser).toDisplayCollapsedRedbox(`
     |                             ^
  69 |        {
  70 |          "count": 1,
  71 |          "description": "[ Server ] Error: Route "/error" used \`Math.random()\` outside of \`"use cache"\` and without explicitly calling \`await connection()\` beforehand. See more info here: https://nextjs.org/docs/messages/next-prerender-random",

  at Object.toDisplayCollapsedRedbox (development/app-dir/dynamic-io-dev-errors/dynamic-io-dev-errors.test.ts:68:29)

● Dynamic IO Dev Errors › should display error when component accessed data without suspense boundary

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `Dynamic IO Dev Errors should display error when component accessed data without suspense boundary 1`

- Snapshot  - 1
+ Received  + 1

@@ -6,10 +6,10 @@
      "Page [Server] <anonymous> (2:1)",
      "main <anonymous> (2:1)",
      "body <anonymous> (2:1)",
      "html <anonymous> (2:1)",
      "Root [Server] <anonymous> (2:1)",
-     "<FIXME-internal-frame>",
      "JSON.parse <anonymous> (0:0)",
+     "<unknown> <anonymous> (0:0)",
    ],
    "title": "Console Error",
  }

  141 |       `)
  142 |     } else {
> 143 |       await expect(browser).toDisplayCollapsedRedbox(`
      |                             ^
  144 |        {
  145 |          "count": 1,
  146 |          "description": "[ Server ] Error: Route "/no-accessed-data": A component accessed data, headers, params, searchParams, or a short-lived cache without a Suspense boundary nor a "use cache" above it. We don't have the exact line number added to error messages yet but you can see which component in the stack below. See more info: https://nextjs.org/docs/messages/next-prerender-missing-suspense",

  at Object.toDisplayCollapsedRedbox (development/app-dir/dynamic-io-dev-errors/dynamic-io-dev-errors.test.ts:143:29)

● Dynamic IO Dev Errors › should clear segment errors after correcting them

page.waitForSelector: Timeout 5000ms exceeded.
Call log:
  - waiting for locator('[data-nextjs-toast]')

  423 |     return this.chain(() => {
  424 |       return page
> 425 |         .waitForSelector(selector, { timeout, state: 'attached' })
      |          ^
  426 |         .then(async (el) => {
  427 |           // it seems selenium waits longer and tests rely on this behavior
  428 |           // so we wait for the load event fire before returning

  at waitForSelector (lib/browsers/playwright.ts:425:10)

Read more about building and testing Next.js in contributing.md.

pnpm test-dev-turbo test/e2e/app-dir/server-source-maps/server-source-maps.test.ts (turbopack)

  • app-dir - server source maps > thrown SSR errors
Expand output

● app-dir - server source maps › thrown SSR errors

expect(received).toMatchInlineSnapshot(snapshot)

Snapshot name: `app-dir - server source maps thrown SSR errors 1`

- Snapshot  - 9
+ Received  + 6

  {
+   "count": 1,
    "description": "Error: Boom",
    "source": "app/ssr-throw/Thrower.js (4:9) @ throwError
-
-   2 |
-   3 | function throwError() {
  > 4 |   throw new Error('Boom')
-     |         ^
-   5 | }
-   6 |
-   7 | export function Thrower() {",
-   "stack": "Thrower
- app/ssr-throw/Thrower.js (8:3)",
+     |         ^",
+   "stack": [
+     "Thrower app/ssr-throw/Thrower.js (8:3)",
+   ],
+   "title": "Unhandled Runtime Error",
  }

  201 |
  202 |       if (isTurbopack) {
> 203 |         await expect(browser).toDisplayRedbox(`
      |                               ^
  204 |          {
  205 |            "description": "Error: Boom",
  206 |            "source": "app/ssr-throw/Thrower.js (4:9) @ throwError

  at Object.toDisplayRedbox (e2e/app-dir/server-source-maps/server-source-maps.test.ts:203:31)

Read more about building and testing Next.js in contributing.md.

@ijjk
Copy link
Member

ijjk commented Nov 25, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
buildDuration 21.3s 19.7s N/A
buildDurationCached 18.5s 15.6s N/A
nodeModulesSize 409 MB 409 MB N/A
nextStartRea..uration (ms) 542ms 544ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
1187-HASH.js gzip 50.4 kB 50.4 kB N/A
8276.HASH.js gzip 169 B 168 B N/A
8377-HASH.js gzip 5.3 kB 5.3 kB N/A
bccd1874-HASH.js gzip 53 kB 53 kB N/A
framework-HASH.js gzip 57.5 kB 57.5 kB N/A
main-app-HASH.js gzip 233 B 235 B N/A
main-HASH.js gzip 33.8 kB 33.7 kB N/A
webpack-HASH.js gzip 1.71 kB 1.71 kB N/A
Overall change 0 B 0 B
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
polyfills-HASH.js gzip 39.4 kB 39.4 kB
Overall change 39.4 kB 39.4 kB
Client Pages
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
_app-HASH.js gzip 193 B 193 B
_error-HASH.js gzip 193 B 193 B
amp-HASH.js gzip 512 B 510 B N/A
css-HASH.js gzip 343 B 342 B N/A
dynamic-HASH.js gzip 1.84 kB 1.84 kB
edge-ssr-HASH.js gzip 265 B 265 B
head-HASH.js gzip 363 B 362 B N/A
hooks-HASH.js gzip 393 B 392 B N/A
image-HASH.js gzip 4.44 kB 4.43 kB N/A
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.35 kB 2.34 kB N/A
routerDirect..HASH.js gzip 328 B 328 B
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 323 B 326 B N/A
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 3.59 kB 3.59 kB
Client Build Manifests
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
_buildManifest.js gzip 747 B 745 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
index.html gzip 524 B 523 B N/A
link.html gzip 539 B 537 B N/A
withRouter.html gzip 520 B 520 B
Overall change 520 B 520 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
edge-ssr.js gzip 128 kB 128 kB N/A
page.js gzip 203 kB 203 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
middleware-b..fest.js gzip 669 B 668 B N/A
middleware-r..fest.js gzip 155 B 156 B N/A
middleware.js gzip 31 kB 31 kB N/A
edge-runtime..pack.js gzip 844 B 844 B
Overall change 844 B 844 B
Next Runtimes
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
523-experime...dev.js gzip 322 B 322 B
523.runtime.dev.js gzip 314 B 314 B
app-page-exp...dev.js gzip 322 kB 322 kB
app-page-exp..prod.js gzip 127 kB 127 kB
app-page-tur..prod.js gzip 140 kB 140 kB
app-page-tur..prod.js gzip 135 kB 135 kB
app-page.run...dev.js gzip 312 kB 312 kB
app-page.run..prod.js gzip 122 kB 122 kB
app-route-ex...dev.js gzip 37.1 kB 37.1 kB
app-route-ex..prod.js gzip 25.1 kB 25.1 kB
app-route-tu..prod.js gzip 25.1 kB 25.1 kB
app-route-tu..prod.js gzip 24.9 kB 24.9 kB
app-route.ru...dev.js gzip 38.7 kB 38.7 kB
app-route.ru..prod.js gzip 24.9 kB 24.9 kB
pages-api-tu..prod.js gzip 9.56 kB 9.56 kB
pages-api.ru...dev.js gzip 11.4 kB 11.4 kB
pages-api.ru..prod.js gzip 9.56 kB 9.56 kB
pages-turbo...prod.js gzip 21.3 kB 21.3 kB
pages.runtim...dev.js gzip 27 kB 27 kB
pages.runtim..prod.js gzip 21.3 kB 21.3 kB
server.runti..prod.js gzip 916 kB 916 kB N/A
Overall change 1.43 MB 1.43 MB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them Change
0.pack gzip 2.03 MB 2.04 MB ⚠️ +10.1 kB
index.pack gzip 71.7 kB 72 kB ⚠️ +298 B
Overall change 2.11 MB 2.12 MB ⚠️ +10.4 kB
Diff details
Diff for main-HASH.js

Diff too large to display

Diff for server.runtime.prod.js

Diff too large to display

Commit: a4686e5

@eps1lon eps1lon force-pushed the sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them branch 4 times, most recently from a5f308d to dfcbb44 Compare November 25, 2024 18:13
# Better than Datadog's default (e.g. https://github.com/vercel/next.js/commit/3dd3d19ba63c7ca790f3bf39e0e15152b597547c/checks)
# Job id is not provided by GitHub. Using workflow url as a fallback.
# We could derived the job id from GH API response but this is just more network slowness.
DD_CI_JOB_URL: https://github.com/${{ github.repository }}/actions/runs/${{ github.run_id }}/attempts/${{ github.run_attempt }}
Copy link
Member Author

Choose a reason for hiding this comment

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

Ignored for unknown reasons: DataDog/datadog-ci#1497

@eps1lon eps1lon force-pushed the sebbie/11-20-track_errors_where_frames_are_not_sourcemapped_properly_in_the_redbox branch 2 times, most recently from 4b21ffe to 991d29c Compare November 25, 2024 20:57
@devjiwonchoi devjiwonchoi force-pushed the sebbie/11-20-track_errors_where_frames_are_not_sourcemapped_properly_in_the_redbox branch 7 times, most recently from 18c98bc to 9e6d906 Compare December 4, 2024 14:41
@eps1lon eps1lon force-pushed the sebbie/11-20-track_errors_where_frames_are_not_sourcemapped_properly_in_the_redbox branch 2 times, most recently from 367992f to f11c84f Compare December 8, 2024 15:33
@eps1lon eps1lon force-pushed the sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them branch from dfcbb44 to 64d5339 Compare December 10, 2024 23:04
@ijjk ijjk added create-next-app Related to our CLI tool for quickly starting a new Next.js application. Documentation Related to Next.js' official documentation. examples Issue was opened via the examples template. labels Dec 10, 2024
@ijjk ijjk added Font (next/font) Related to Next.js Font Optimization. tests Turbopack Related to Turbopack with Next.js. type: next labels Dec 10, 2024
@eps1lon eps1lon changed the base branch from sebbie/11-20-track_errors_where_frames_are_not_sourcemapped_properly_in_the_redbox to canary December 10, 2024 23:04
@eps1lon eps1lon force-pushed the sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them branch from 64d5339 to a4686e5 Compare December 10, 2024 23:18
@eps1lon eps1lon changed the base branch from canary to sebbie/12-06-add_todisplayredbox_snapshot_matcher December 10, 2024 23:18
@eps1lon eps1lon force-pushed the sebbie/12-06-add_todisplayredbox_snapshot_matcher branch from 7b303e2 to e279a4a Compare December 11, 2024 00:45
Previously, each test run was associated with the Job
that uploaded them.
This makes it harder to understand under which configuration
a specific test failed.
Now we immediately upload test results from the
respective job.

This increases feedback time slightly by up to 1 minute
(the slowest single upload time).
I'd say this is an acceptable tradeoff given our
current CI times.

This reduces the time until a "Retry failed jobs"
is avaiable since we no longer need to pass the test
results between jobs.
@eps1lon eps1lon force-pushed the sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them branch from a4686e5 to 4b1b811 Compare December 11, 2024 12:30
@eps1lon
Copy link
Member Author

eps1lon commented Dec 11, 2024

Blocked by DataDog/datadog-ci#1497

@eps1lon eps1lon closed this Dec 11, 2024
@eps1lon eps1lon deleted the sebbie/11-25-report_tests_to_datadog_from_job_that_executed_them branch December 11, 2024 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
create-next-app Related to our CLI tool for quickly starting a new Next.js application. created-by: Next.js team PRs by the Next.js team. Documentation Related to Next.js' official documentation. examples Issue was opened via the examples template. Font (next/font) Related to Next.js Font Optimization. tests Turbopack Related to Turbopack with Next.js. type: next
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants