feat(html reporter): show attachments in attachment steps

[Skn0tt]

Closes #32748.

Adds a TestStep.attachments array to the Reporter API that shows all attachments for the step. Removes the artificial attach "foo" step for all calls except TestInfo.attach(). Updates the HTML Report to show attachments in the step list.

Before:

After:

Note that the Attachments chip goes away in favour of showing all attachments in the steps view. This means there's less repetition in the UI, but it also makes it harder to find deeply nested attachments. I'll also work on an alternative UI to see what else we could show here.

[Skn0tt]

Those test failures are all because attachments are now listed twice on the page, so Playwright doesn't know what element to resolve to. I'll update the tests once we've agreed on a UI for this.

[Skn0tt]

Turning into draft to implement what we discussed yesterday:

• TestStep.attachments return its own attachments
• remove attachment steps for everything but explicit TestInfo.attach()
• add UI for linking from steps to attachments to Trace Viewer and HTML report

[Skn0tt]

Here's another draft for how the attachments UI could look like. It keeps the existing Attachments chip, and links to it from the step list:

Screen.Recording.2024-10-16.at.16.02.48.mov

It's not very polished, but I think I prefer the current approach where the attachments are inside the step list.

code

[Skn0tt]

notes from call with Dima:

• "attach" step stays, but not for attachments.push. give folks a reason to migrate to .attach
• in trace viewer, "attach" step gets attachment icon
• attachment icon somehow reveals attachment bodies (either instead of browser snapshot or in attachment tab)
• attachment tab has some sort of backlink to sources or actions (make it similar to errors)
• that way, trace structure doesn't need any changes
• open question: is attachment icon recursive? does it show the icon if child steps have attachments?
• for toHaveScreenshot: make sure that they don't have their own "attach" steps
• our own steps shouldn't rely on zones for parent detection, make it explicit
  • reuse step.complete

Unrelated:

• "expect.toHaveScreenshot" attachment shouldn't say "image diff", but have a title that references to toHaveScreenshot. Ideally another backlink

[github-actions]

Test results for "tests 1"

55 failed
❌ [playwright-test] › playwright.trace.spec.ts:516:5 › should include attachments by default @macos-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:546:5 › should opt out of attachments @macos-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:718:5 › should not throw when attachment is missing @macos-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:737:5 › should not throw when screenshot on failure fails @macos-latest-node18-1
❌ [playwright-test] › to-have-screenshot.spec.ts:236:5 › should report toHaveScreenshot step with expectation name in title @macos-latest-node18-1
❌ [playwright-test] › ui-mode-trace.spec.ts:76:5 › should merge screenshot assertions @macos-latest-node18-1
❌ [playwright-test] › ui-mode-trace.spec.ts:180:5 › should show screenshot @macos-latest-node18-1
❌ [playwright-test] › reporter-html.spec.ts:342:5 › created › should not include image diff with non-images @macos-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:843:5 › created › should have link for opening HTML attachments in new tab @macos-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:869:5 › created › should use file-browser friendly extensions for buffer attachments based on contentType @macos-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:917:5 › created › should show attachments in step lsit @macos-latest-node18-2
❌ [playwright-test] › playwright.trace.spec.ts:516:5 › should include attachments by default @ubuntu-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:546:5 › should opt out of attachments @ubuntu-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:718:5 › should not throw when attachment is missing @ubuntu-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:737:5 › should not throw when screenshot on failure fails @ubuntu-latest-node18-1
❌ [playwright-test] › to-have-screenshot.spec.ts:236:5 › should report toHaveScreenshot step with expectation name in title @ubuntu-latest-node18-1
❌ [playwright-test] › ui-mode-trace.spec.ts:76:5 › should merge screenshot assertions @ubuntu-latest-node18-1
❌ [playwright-test] › ui-mode-trace.spec.ts:180:5 › should show screenshot @ubuntu-latest-node18-1
❌ [playwright-test] › reporter-html.spec.ts:342:5 › created › should not include image diff with non-images @ubuntu-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:843:5 › created › should have link for opening HTML attachments in new tab @ubuntu-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:869:5 › created › should use file-browser friendly extensions for buffer attachments based on contentType @ubuntu-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:917:5 › created › should show attachments in step lsit @ubuntu-latest-node18-2
❌ [playwright-test] › playwright.trace.spec.ts:516:5 › should include attachments by default @ubuntu-latest-node20-1
❌ [playwright-test] › playwright.trace.spec.ts:546:5 › should opt out of attachments @ubuntu-latest-node20-1
❌ [playwright-test] › playwright.trace.spec.ts:718:5 › should not throw when attachment is missing @ubuntu-latest-node20-1
❌ [playwright-test] › playwright.trace.spec.ts:737:5 › should not throw when screenshot on failure fails @ubuntu-latest-node20-1
❌ [playwright-test] › to-have-screenshot.spec.ts:236:5 › should report toHaveScreenshot step with expectation name in title @ubuntu-latest-node20-1
❌ [playwright-test] › ui-mode-trace.spec.ts:76:5 › should merge screenshot assertions @ubuntu-latest-node20-1
❌ [playwright-test] › ui-mode-trace.spec.ts:180:5 › should show screenshot @ubuntu-latest-node20-1
❌ [playwright-test] › reporter-html.spec.ts:342:5 › created › should not include image diff with non-images @ubuntu-latest-node20-2
❌ [playwright-test] › reporter-html.spec.ts:843:5 › created › should have link for opening HTML attachments in new tab @ubuntu-latest-node20-2
❌ [playwright-test] › reporter-html.spec.ts:869:5 › created › should use file-browser friendly extensions for buffer attachments based on contentType @ubuntu-latest-node20-2
❌ [playwright-test] › reporter-html.spec.ts:917:5 › created › should show attachments in step lsit @ubuntu-latest-node20-2
❌ [playwright-test] › playwright.trace.spec.ts:516:5 › should include attachments by default @ubuntu-latest-node22-1
❌ [playwright-test] › playwright.trace.spec.ts:546:5 › should opt out of attachments @ubuntu-latest-node22-1
❌ [playwright-test] › playwright.trace.spec.ts:718:5 › should not throw when attachment is missing @ubuntu-latest-node22-1
❌ [playwright-test] › playwright.trace.spec.ts:737:5 › should not throw when screenshot on failure fails @ubuntu-latest-node22-1
❌ [playwright-test] › to-have-screenshot.spec.ts:236:5 › should report toHaveScreenshot step with expectation name in title @ubuntu-latest-node22-1
❌ [playwright-test] › ui-mode-trace.spec.ts:76:5 › should merge screenshot assertions @ubuntu-latest-node22-1
❌ [playwright-test] › ui-mode-trace.spec.ts:180:5 › should show screenshot @ubuntu-latest-node22-1
❌ [playwright-test] › reporter-html.spec.ts:342:5 › created › should not include image diff with non-images @ubuntu-latest-node22-2
❌ [playwright-test] › reporter-html.spec.ts:843:5 › created › should have link for opening HTML attachments in new tab @ubuntu-latest-node22-2
❌ [playwright-test] › reporter-html.spec.ts:869:5 › created › should use file-browser friendly extensions for buffer attachments based on contentType @ubuntu-latest-node22-2
❌ [playwright-test] › reporter-html.spec.ts:917:5 › created › should show attachments in step lsit @ubuntu-latest-node22-2
❌ [playwright-test] › playwright.trace.spec.ts:516:5 › should include attachments by default @windows-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:546:5 › should opt out of attachments @windows-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:718:5 › should not throw when attachment is missing @windows-latest-node18-1
❌ [playwright-test] › playwright.trace.spec.ts:737:5 › should not throw when screenshot on failure fails @windows-latest-node18-1
❌ [playwright-test] › to-have-screenshot.spec.ts:236:5 › should report toHaveScreenshot step with expectation name in title @windows-latest-node18-1
❌ [playwright-test] › ui-mode-trace.spec.ts:76:5 › should merge screenshot assertions @windows-latest-node18-1
❌ [playwright-test] › ui-mode-trace.spec.ts:180:5 › should show screenshot @windows-latest-node18-1
❌ [playwright-test] › reporter-html.spec.ts:342:5 › created › should not include image diff with non-images @windows-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:843:5 › created › should have link for opening HTML attachments in new tab @windows-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:869:5 › created › should use file-browser friendly extensions for buffer attachments based on contentType @windows-latest-node18-2
❌ [playwright-test] › reporter-html.spec.ts:917:5 › created › should show attachments in step lsit @windows-latest-node18-2

36354 passed, 639 skipped
✔️✔️✔️

Merge workflow run.

[Skn0tt]

closing in favour of separate PRs