-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Closed
Description
Version
v1.56.0
Steps to reproduce
The bug is in
| this._attachmentsPush = this.attachments.push.bind(this.attachments); |
typescriptthis._attachmentsPush = this.attachments.push.bind(this.attachments);
this.attachments.push = (...attachments: TestInfo['attachments']) => {
for (const a of attachments)
this._attach(a, this._parentStep()?.stepId);
return this.attachments.length;
};
-> direct assignment creates an enumerable property on the array.
- no existing tests compare test.info().attachments directly with toEqual([]);
- tests only compare individual attachment objects or arrays from reports;
- bug was discovered when implementing the same pattern for annotations.push, which does have direct equality tests.
Expected behavior
- wrapping arr methods should not affect arr eq checks;
- Object.keys(testInfo.attachments) should return [] for an empty attachments arr;
- expect(testInfo.attachments).toEqual([]) should pass for empty attachments;
- push should be non-enumerable (like built-in arr methods).
Actual behavior
- direct assignment makes the push enumerable;
- Object.keys(testInfo.attachments) returns ['push'] even for empty arrs;
- tests toEqual() sees the enumerable property and fails with: "Received: serializes to the same string";
- tests that compare testInfo.attachments directly will fail unexpectedly.
Additional context
example of how it should be handled https://github.com/microsoft/playwright/pull/37761/files#diff-4f61694176d25f70ab6fc3d94b43ef85e9c178d8ecc5680268046e534db1515fR223
Environment
System:
OS: Linux 6.2 Ubuntu 22.04.3 LTS 22.04.3 LTS (Jammy Jellyfish)
CPU: (8) arm64
Binaries:
Node: 18.19.0 - ~/.nvm/versions/node/v18.19.0/bin/node
npm: 10.2.3 - ~/.nvm/versions/node/v18.19.0/bin/npm
npmPackages:
@playwright/test: 1.56.0 => 1.56.0Metadata
Metadata
Assignees
Labels
No labels