[Docs]: Discourage `Download.createReadStream`

[WiseNat]

Page(s)

https://playwright.dev/docs/downloads

Description

The Downloads documentation is rather limited surrounding the createReadStream method. I appreciate the Readable Object is not a Playwright Object but information on how to block Playwright so that file content assertions can be performed would be useful.

When I naively created a test with the Readable Object documentations recommended way of reading content, Playwright did not wait for the stream to finish reading and would show false-positives with tests passing. This is visible with the following:

  // Act
  const [download] = await Promise.all([
    page.waitForEvent("download"),
    // Trigger a file download
  ]);

  const readable = await download.createReadStream();
  readable.setEncoding("binary");

  const chunks: string[] = [];

  readable.on("readable", () => {
    let chunk;
    while (null !== (chunk = readable.read())) {
      chunks.push(chunk);
    }
  });

  readable.on("end", () => {
    const actual = chunks.join("");
    const expected = "Example File Content";

    expect(actual).toEqual(expected);
    expect("A").toEqual("B"); // Should fail for all tests
  });

The expect("A").toEqual("B"); in the "end" event should cause all tests to fail. However, both "readable" and "end" events are asynchronous and so Playwright does not block for them. So, when reading the file data into chunks, Playwright will simultaneously close leading to the following error:
Error: readableStreamImpl._read: Target page, context or browser has been closed

The way I've managed to get this to work is with the following:

// Act
const [download] = await Promise.all([
  page.waitForEvent("download"),
  // Trigger a file download
]);

const readable = await download.createReadStream();
readable.setEncoding("binary");

let actual = "";
readable.on("data", (chunk) => (actual += chunk));

await finished(readable);

const expected = "Example File Content";

expect(actual).toEqual(expected);

With await finished(readable);, Playwright is blocked, removing the flakiness of testing the contents of a downloaded file.

I appreciate people may come to the same conclusion as me, in hindsight it's fairly obvious why the flakiness occurred, but adding information for asserting on a downloaded file's content to the documentation would help people testing this.

[Skn0tt]

We perceive createReadStream as an advanced feature that's mostly used by library users, not meant for testing. That's why we assume that users have a good understanding of createReadStream and Readable. Those APIs are finicky, and I worry that if we improved docs, more people would use it, and more people would run into its footguns.

In your example, you could've performed a file system read from the Download#path() path. Would that have worked for your usecase? Maybe we should put a warning into the docs and point there.

[WiseNat]

I've tried on my end and reading the file off the back of Download#path() works fine for the testing I'm doing. It also makes the code significantly easier to follow so thanks for the suggestion.

I definitely agree that it'd be worth mentioning something about that in the documentation; from my perspective, I saw the createReadStream method and presumed that was the intended way of reading file data when testing.