playwright icon indicating copy to clipboard operation
playwright copied to clipboard

[Feature] beforeEach/fixtures, but executed even for skipped tests

Open throw5way opened this issue 1 year ago • 5 comments

There is no way to execute code for every test, even those defined with test.skip('', () => {}).

This is required in order to customize reporting for skipped tests. For other tests it can be done using a fixture or with beforeEach hook, but skipped tests execute neither of those, and no customization, such as assigning labels in Allure dependent on test location, is possible.

throw5way avatar Jan 30 '24 01:01 throw5way

Previous discussion asked to refile issue if persists. It does.

throw5way avatar Jan 30 '24 01:01 throw5way

Depending on whether you want the test body (along with the fixtures and ability to add annotations) to run for the skipped tests, or not run for the skipped tests, you should use one of the strategies:

  1. to run the test body, declare the test as test('foo', { ... }). Then call test.skip() inside the test, after you added the annotations.

  2. to not run the test body, declare the test as test.skip()

Does this address your issue?

pavelfeldman avatar Jan 30 '24 02:01 pavelfeldman

Not at all, sorry.

The problem is exactly that we have to use test.skip(); inside of the test, and that's not the first thing developer does. Everyone skips the test itself, and it doesn't show up in the report. Worse, it isn't even feasible to make a linter rule for it, because of .extend. Fixing it in a funny way with test.skip = (...args) => { ... } doesn't work either, because PW peeks into the syntax of call site.

While I managed to partially and precariously handle it by proxying reporter and duplicating the label logic in onTestBegin, it doesn't solve the problem either, because, unlike fixtures, there is no way to annotate a singular test itself with extra data on how it should be reported.

What I'm asking is literally an ability to run some code on the test itself that is not affected by its expectedStatus.

An { always: true } (similar to { scope: 'worker' } or { auto: true }) flag for a fixture would be the best option.

throw5way avatar Jan 31 '24 22:01 throw5way

It sounds like I did not understand the issue then. Could you provide the exact snippet that I can run locally, along with your expectations and the description of the actual behavior that you are seeing?

pavelfeldman avatar Jan 31 '24 23:01 pavelfeldman

import { test as base } from '@playwright/test';
import { allure } from 'allure-playwright';

const modifyLabels = [
  async ({ }, use, testInfo) => {
    allure.layer('system');
    await use();
  },
  { auto: true },
]

const test = base.extend({
  modifyLabels,
});

test.skip('foo', () => {
  // this test isn't at system layer, because it's skipped
});

test('bar', () => {
  test.skip();
  // this test is at system layer, but developers don't write it this way
});

base('baz', () => {
  // this test is in the same file, but uses different test function
  // if you set all tests to system layer in reporter proxy, it has to go in a different file and different PW config
});

In general there is no way to reliably annotate tests if they are skipped.

What I propose is

const modifyLabels = [
  async ({ }, use, testInfo) => {
    allure.layer('system');
    await use();
  },
  { always: true },
]

test.skip('foo', () => {
  // fixture was executed, body of the test wasn't
});

throw5way avatar Feb 01 '24 12:02 throw5way

@dgozman @pavelfeldman This issue might have been accidentally closed. Linked solution doesn't solve the problem.

If we were to use annotations instead of fixtures here, it'd take several extra lines of code per every test in repository.

I don't see how that might make the code any more readable compared to calling test.skip();.

throw5way avatar Mar 26 '24 08:03 throw5way

Hey @dgozman , @pavelfeldman , we would exactly have the same use case as described by @throw5way. We currently have all teams specify the test metadata like author, ticket, app, etc. in the test.use block, and then have an auto-test fixture to read those properties, verify them, and set them as annotations to pass to our custom reporters.

Sadly, this does not work for skipped tests. As discussed above, the auto fixture body will not get executed, and therefore, we are missing all the metadata information for skipped tests, which is not really good.

The perfect solution for us would be exactly what @throw5way described here. So, to have a fixture option "always" that gets executed no matter the test resolution. Copy of the proposed solution:

const modifyLabels = [
  async ({ }, use, testInfo) => {
    allure.layer('system');
    await use();
  },
  { always: true },
]

test.skip('foo', () => {
  // fixture was executed, body of the test wasn't
}); 

Thomas-Haider avatar Jul 05 '24 05:07 Thomas-Haider