[Regression]: If fixture timeouts in teardown phase, no other fixtures run about playwright HOT 4 OPEN

@vitalets Thank you for the issue!

In general, this is the intended behavior. Once a fixture times out, we run out of time budget to teardown other fixtures. Previously, this worked in a limited number of scenarios, where we would teardown the worker and give it some more time to teardown the fixtures. This only worked when the first fixture won't stall forever, but actually finish given a bit more time, which makes it even less useful. Also, this does not play nicely with things like afterAll() that need all fixtures to be teared down before they can run, and so we cannot introduce extra timeouts there.

It would be nice to understand your usecase - why do you rely on the second fixture teardown to be executed? Perhaps we can improve the situation a bit when there are no hooks, similarly to the old behavior. However, the second timeout will definitely prevent anything else from running.

from playwright.

@dgozman thanks for the explanation!

My use-case is for playwright-bdd project: after running all user-defined fixtures I always need to attach some metadata to the testInfo. This is needed for the correct reporting, even after timeout.

I've also tried to provide a timeout option for my fixture, but it does not help:

export const test = base.extend<{timeoutedFixture: void, anotherFixture: void}>({
  timeoutedFixture: async ({anotherFixture}, use, testInfo) => {
    await use();
    await new Promise((r) => setTimeout(r, testInfo.timeout + 100));
  },
  anotherFixture: [async ({}, use) => {
    console.log('AnotherFixture: setup');
    await use();
    console.log('AnotherFixture: teardown'); // <-- I expect this to run, b/c fixture has own timeout and should not care about other fixtures timeouts
  }, { timeout: 10000 }],
});

Now I'm thinking about two possible solutions:

1. perform empty attachment at the setup phase, and then modify this attachment in-place. Is it reliable enough?
2. have some synchronous way to pass data to the reporter, see #30179

For the more common case I think this could be a trade-off: for some users time budget is more important, but for others it's more important to cleanup fixtures that already passed the setup phase, even if it exceeds the timeout. Maybe there can be an option for that?

from playwright.

I've also tried to provide a timeout option for my fixture, but it does not help

This is an interesting idea! Having a separate timeout would resolve concerns about running out of the time budget. I'll take a stab at implementing this.

but for others it's more important to cleanup fixtures that already passed the setup phase, even if it exceeds the timeout. Maybe there can be an option for that?

Yeah, I think the custom fixture timeout is a good enough option that works here 😄

perform empty attachment at the setup phase, and then modify this attachment in-place. Is it reliable enough?

Not sure what exactly do you mean. We support attaching strings or blobs, which are immutable, so you would only be able to rewrite the value somewhere in test runner internals and I'd advise against that.

have some synchronous way to pass data to the reporter,

Again, not sure how the synchronous aspect helps here, unfortunately.

from playwright.

This is an interesting idea! Having a separate timeout would resolve concerns about running out of the time budget. I'll take a stab at implementing this.

Looking forward for it!

perform empty attachment at the setup phase, and then modify this attachment in-place. Is it reliable enough?

Not sure what exactly do you mean. We support attaching strings or blobs, which are immutable, so you would only be able to rewrite the value somewhere in test runner internals and I'd advise against that.

I mean searching for attachment in testInfo.attachments and modifying it's content. E.g.:

function updateAttachment(name, body) {
  const attachment = testInfo.attachments.find(a => a.name === name);
  attachment.body = body;
}

I can call updateAttachment() after each step during test execution, and then there is no need to finally call testInfo.attach() in fixture teardown. But this is relevant only for my use-case and does not solve this timeout issue in general. And I've checked, modifying attachments does not work 😁

from playwright.