[Regression]: testInfo.attach() and testInfo.attachments.push() produce different reporter data

[vitalets]

Last Good Version

1.49

First Bad Version

1.50

Steps to reproduce

Follow the steps in https://github.com/vitalets/playwright-issues/tree/testinfo-attachments-consistency.
Actually it uses test example from #34037, this test.

Expected behavior

I expect the behavior of testInfo.attach() and testInfo.attachments.push() is consistent: both produce equal reporter data structure.

Actual behavior

• testInfo.attach() adds extra step attach "foo-attach"
• testInfo.attachments.push() does not add extra step attach "foo-push", it stores attachment directly in step.attachments.

Additional context

I've read the discussion in #34037 but still not sure, is it expected behavior with new step.attachments field.
In Playwright 1.49 both methods of adding attachments behave equally.
Maybe it's worth highlighting the difference in the docs.

Environment

System:
    OS: macOS 14.7.2
    CPU: (10) arm64 Apple M1 Pro
    Memory: 79.66 MB / 16.00 GB
  Binaries:
    Node: 20.11.1 - ~/.nvm/versions/node/v20.11.1/bin/node
    npm: 10.2.4 - ~/.nvm/versions/node/v20.11.1/bin/npm
  Languages:
    Bash: 3.2.57 - /bin/bash
  npmPackages:
    @playwright/test: ^1.50.0-beta-1737179657000 => 1.50.0-beta-1737179657000

[Skn0tt]

Yes, this change is intended. The docs already discourage pushing onto testInfo.attachments:

To add an attachment, use testInfo.attach() instead of directly pushing onto this array.
https://playwright.dev/docs/api/class-testinfo#test-info-attachments

[vitalets]

Per the #34037 description I thought attach "foo" steps should dissapear.

the icon button goes from the removed attach "foo" step to their parent step

Is it correct that they disappear only for attachments.push()?

[Skn0tt]

Yes, that was the intention.