Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Regression]: Can't call test.use() inside a describe() suite of a different test #29734

Closed
Slessi opened this issue Feb 29, 2024 · 58 comments · Fixed by penpot/penpotqa#67 or #33002
Closed

Comments

@Slessi
Copy link

Slessi commented Feb 29, 2024

Last Good Version

1.41.2

First Bad Version

1.42.0

Steps to reproduce

After updating to playwright 1.42, I get this message about one of my test files which was fine in 1.41.2:

Can't call test.use() inside a describe() suite of a different test type.
Make sure to use the same "test" function (created by the test.extend() call) for all declarations inside a suite.playwright

To reproduce, just have describe blocks with extend/use inside of them.

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

base.describe('Admin user', () => {
  // Create an admin item
  const test = base.extend({ item: async ({}, use) => {} });

  /***** Here is the line that triggers errors *****/
  test.use({ storageState: '/admin/user/path' });

  // Tests with admin item
  test('1', async ({ page, item }) => {});
  test('2', async ({ page, item }) => {});
});

base.describe('Normal user', () => {
  // Create a normal item
  const test = base.extend({ item: async ({}, use) => {} });

  /***** Here is the line that triggers errors *****/
  test.use({ storageState: '/normal/user/path' });

  // Tests with normal item
  test('1', async ({ page, item }) => {});
  test('2', async ({ page, item }) => {});
});

Expected behavior

I expect to be allowed to have use/extend inside a describe block.

Actual behavior

I am not allowed to have use/extend inside a describe block.

Additional context

No response

Environment

System:
  OS: macOS 14.3.1
  CPU: (10) arm64 Apple M1 Pro
  Memory: 3.52 GB / 32.00 GB
Binaries:
  Node: 20.11.1 - ~/.nvm/versions/node/v20.11.1/bin/node
  Yarn: 1.22.21 - /opt/homebrew/bin/yarn
  npm: 10.2.4 - ~/.nvm/versions/node/v20.11.1/bin/npm
IDEs:
  VSCode: 1.86.2 - /opt/homebrew/bin/code
Languages:
  Bash: 3.2.57 - /bin/bash
npmPackages:
  @playwright/test: ^1.42.0 => 1.42.0
@pavelfeldman
Copy link
Member

Allowing the following code was an oversight on our side. Mixing the test instances (and hooks called on those instances) in the same suite prevents us from keeping the strict semantic.

const test = base.extend({ item: async ({}, use) => {} });
base.describe('Admin user', () => {
  test('1', async ({ page, item }) => {});
  test('2', async ({ page, item }) => {});
});

I'll leave this change open to see how many people are affected by this.

@pavelfeldman
Copy link
Member

I updated the release notes to include this as a breaking change. Is there a reason you need to mix test instances? Can you fix the code?

@digitalclubb
Copy link

digitalclubb commented Feb 29, 2024

Also affected here. I've popped a question on the Discord group but cross posting for visibility:

I have the following setup for visual testing:

describe('goes to confirm page', () => {
    beforeEach(async ({ page }) => {
        await formToConfirmation(page);
        await form.titlePage.waitFor();
    });
    
    test('@visual desktop', async ({ captureScreenshot }) => {
        await captureScreenshot('aml-contacts-desktop-confirm');
    });

    testMobile('@visual mobile', async ({ captureScreenshot }) => {
        await captureScreenshot('aml-contacts-mobile-confirm');
    });
});

testMobile comes from our playwright.config.ts file:

export const testMobile = test.extend<PlaywrightTestArgs>({
    viewport: { width: 414, height: 896 },
});

We've found this a nice way to manage our tests, however is the expectation now to have a separate describe block when the test instances change? It's a shame as this seemed to offer quite a maintainable approach to our testing at scale.

@Slessi
Copy link
Author

Slessi commented Feb 29, 2024

I updated the release notes to include this as a breaking change. Is there a reason you need to mix test instances? Can you fix the code?

@pavelfeldman There is no particular reason, I just thought it looked neat the way I wrote it. Since it wasn't mentioned in the release notes I wasn't sure if it was an intentional change or not.

I can also do this, it was only to avoid having fixtures like adminItem and normalItem without splitting into seperate files, so I nested the fixtures to have them both be item.

(If it is still wrong, since I have different test.use per describe block, please let me know.)

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

const test = base.extend({
  adminItem: async ({}, use) => {},
  normalItem: async ({}, use) => {},
});

test.describe('Admin user', () => {
  test.use({ storageState: '/admin/user/path' });

  test('1', async ({ page, adminItem }) => {});
  test('2', async ({ page, adminItem }) => {});
});

test.describe('Normal user', () => {
  test.use({ storageState: '/normal/user/path' });

  test('1', async ({ page, normalItem}) => {});
  test('2', async ({ page, normalItem }) => {});
});

@nagr01
Copy link

nagr01 commented Mar 1, 2024

We're affected by this change as well

@gurudev7699
Copy link

gurudev7699 commented Mar 1, 2024

We are experiencing difficulties due to this recent update, and we'd like to share our code snippet to highlight its repercussions. Adapting to this change requires significant effort, as it's disrupting the functionality of nearly all our tests.

The error we get is

Error: Can't call test() inside a describe() suite of a different test type.
Make sure to use the same "test" function (created by the test.extend() call) for all declarations inside a suite.

Our setup involves categorizing shows based on user access or the presence of a paywall, or sometimes both. This feature is critical not only for our organization but also for every other organization using Playwright. As others begin upgrading to the latest version, the absence of this feature will undoubtedly have a widespread impact on test organization for all users. Therefore, we kindly request the restoration of this feature to ensure smooth operations for all users.

import freeContentShow from 'path/to/freeContentMultipleSeasons.json';
import { expect, test } from 'path/to/test';

const { platform } = config;
const fixtures = {
  free: ['show1', 'show2', 'Season 5'],
  premium: ['showA', 'showB', 'Season 2']
};
const [freeShowUrl, premiumShowUrl, seasonName] = fixtures[platform];

test.describe('Test Description 1', () => {
  test.beforeEach(async ({ page, platform }) => {
    await page.route(`**/catalog/v2/${platform}/show/${freeShowUrl}*`, async (route) => {
      const json = freeContentShow[platform];
      await route.fulfill({ json });
    });
  });
  test('Test Name 1', async ({ page, platform }) => {
    await page.goto(`/${freeShowUrl}`);
    await expect(page.getByLabel(seasonName)).toHaveScreenshot(`free-season-${platform}.png`);
  });
});

test.describe('Test Description 2', () => {
  test.beforeEach(async ({ page, platform }) => {
    await page.route(`**/catalog/v2/${platform}/show/${premiumShowUrl}*`, async (route) => {
      const json = require(`path/to/showAllMediaPremium.json`);
      await route.fulfill({ json });
    });
  });
  test('Test Name 2', async ({ page, platform }) => {
    await page.goto(`/${premiumShowUrl}`);
    await expect(page.getByLabel(seasonName)).toHaveScreenshot(`premium-season-${platform}.png`);
  });
});

@vsharmab
Copy link

vsharmab commented Mar 4, 2024

I am also having issues with this change. I was using this to set up an electron app or browser within the same test suite.

@Kradirhamik
Copy link

We are affected by this change as well.

In our case we have test groupings which, after consuming dynamically the current combination under test, determines a particular version of the test group to be executed.

@man-qa
Copy link

man-qa commented Mar 19, 2024

We're affected by this change as well. we are using under same describe default "test" and modified "test" fixtures to create test groups

@nagr01
Copy link

nagr01 commented Mar 20, 2024

@pavelfeldman Any updates as to whether this breaking change will be reverted? Our playwright testing framework is built around this functionality, and we would very much like to maintain the current way that it is working, At the moment, we're stuck on version 1.41, and we're in a bit of uncertainty as to what the future holds (whether we will have to change our architectural approach completely or whether we should wait to see if this breaking change will be reverted).

@hannahpub
Copy link

Yes, same issue
Error: Can't call test.describe() inside a describe() suite of a different test type
Out automation test using playwright were using two structures of myTest and test, and now it's not working. any suggestions on this? I have to rewire all to separate each?

@mstepin
Copy link

mstepin commented Mar 21, 2024

not sure if it is related or not, but we're on version 1.40

And getting this error:

Cannot use({ fixture1 }) in a describe group, because it forces a new worker.
Make it top-level in the test file or put in the configuration file.

where we try to do something like the code below. We use fixtures quite heavily and they set up a lot of our entities, but lately we've come to scenarios where we would potentially not want them invoked conditionally per test (so can't use a global setting such as auto: true/false.

test.describe(
    'hello world',
    () => {
        if(condition) test.use({ fixture1: '' });
        test.only( 'just testing', async ({ fixture2, fixture1 }) => {
            // ...do some things
        })
    }
)

@lanej0
Copy link

lanej0 commented Mar 22, 2024

Also affected by this change in a number of our tests.

@Heops
Copy link

Heops commented Mar 25, 2024

Faced with this issue too

@Alexbirvik
Copy link

The same issue

@brychter-hippo
Copy link

We are facing same issue in our applications.

@stevejhiggs
Copy link

Also affected by this, we have put off upgrading playwright until a decision is made here

@as-sharubina
Copy link

Facing the same issue

@KimotoYanke
Copy link

We have the same issue.

@Trinovantes
Copy link
Contributor

Trinovantes commented Apr 3, 2024

I don't like having to duplicate code

const test = base.extend({ ... })
const authTest = test.extend({ ... })

// Before
test.describe('desc', () => {
  test.beforeEach(() => { ... })
  test('test', () => { ... })
  authTest('test', () => { ... })
})

// After
test.describe('desc', () => {
  test.beforeEach(() => { ... })
  test('test', () => { ... })
})
authTest.describe('desc (authenticated)', () => {
  authTest.beforeEach(() => { duplicate code })
  authTest('test', () => { ... })
})

Probably not ideal but my workaround is to create a fixture that I load strictly for its side effects whenever I want to modify the page context:

const test = base.extend<{ __AUTH__: Page }>({
  __AUTH__: async({ page }, use) => {
    await page.addInitScript(() => {})
    await use(page)
  },
})

test.describe('desc', () => {
  test.beforeEach(() => { ... })
  test('test', () => { ... })
  test('test', ({ __AUTH__ }) => { await __AUTH__.reload(); ... })
})

Personally I think any extension of a test should be allowed to be nested instead of it. For example:

const A = base.extend({ ... })
const A1 = A.extend({ ... })
const B = base.extend({ ... })

A.describe('', () => {
  A.test('', () => { }) // works as expected
  A1.test('', () => { }) // should be allowed
  B.test('', () => { }) // should not be allowed (?)
})

@iguerrero-rl
Copy link

My company is also affected by this.

@TaylorMichaelHall
Copy link

This also affects some of our utilities

@Dtesch9
Copy link

Dtesch9 commented Apr 11, 2024

Affected, downgraded in order to use the last good version.

@alextompkins
Copy link

Yeah, to me it seems that this is a breaking change and hence shouldn't be included in a minor version regardless of reasoning.

Mixing the test instances (and hooks called on those instances) in the same suite prevents us from keeping the strict semantic.

@pavelfeldman could you please explain what exactly you mean here? Maybe there's another solution.

@lengband
Copy link

We're affected by this change as well, any updates? @pavelfeldman

@lanej0
Copy link

lanej0 commented May 2, 2024

Can we get an update on this? I'm not able to update @playwright/test to the latest because of this breaking change. Is it going to be reverted, or should we start refactoring all of our tests for this? @pavelfeldman

@drewhan90
Copy link

This change also affects a project I am working on.

@orangehb
Copy link

orangehb commented May 7, 2024

This change also affects a project I am working on. +1 It would be really really great if it's fixed in the next release version.

@anujshah3
Copy link

This has affected us toooooooo!

@SagaciousLittle
Copy link

+1, any update about this?

@lengband
Copy link

@mxschmitt Many people are looking forward to fixing this issue, otherwise our existing playwright won't be able to upgrade to the latest version, so keep an eye on it please, thanks

@Dtesch9
Copy link

Dtesch9 commented Jun 20, 2024

+1 waiting...

@AnastasiaOstapova
Copy link

+1

@martynaol
Copy link

affected by this change as well, any updates?

@nagadinesh-git
Copy link

Any workaround for this issue?

@il-darin
Copy link

Probably not helpful to add another "me too", yet here I am :(

@philipfong
Copy link

I'm also affected by this. We're upgrading from 1.40 to 1.45 and it's affecting how our tests are organized.

@kevin-brotcke
Copy link
Contributor

kevin-brotcke commented Jul 25, 2024

This breaks our tests. In our case we have multi-user tests and separate auth test fixtures for each one. So it looks something like:

test.describe("test requiring 2 different users", () => {
  testUserAlice("do something for Bob", ...);
  testUserBob("interact with thing Alice did", ...);
});

We have thousands of tests that depend on this format, not sure how we're going to refactor yet. I could use a single test fixture with test account option but I ran into another issue where you can't call test.use on a single test #27138. If that was implemented it would be a suitable workaround.

Update: I realized after re-reading the docs that my use case is already covered https://playwright.dev/docs/auth#testing-multiple-roles-with-pom-fixtures

So my tests are even better now since they can open both user browser contexts in a single test. For example,

test("test requiring 2 different users", ({ alicePage, bobPage }) => {
  // do tests
});

@mprykhodko
Copy link

+1 affected

@dolkons
Copy link

dolkons commented Aug 19, 2024

+1 affected.
In our project we have multiple custom test instances for different pages. And we want to combine them in one describe block

@mrrinatino
Copy link

+1 affected

@alexmartynenko-powtoon
Copy link

+1 affected. we need to test one feature as different users, also test the same feature which opens above different applications - and it doesnt make sense so split them under different describe.

@ValorHeart
Copy link
Contributor

+1 affected, would be nice to be able to have test.use() per test, in a single spec file

@MartinSchmidt65
Copy link

We are also affected by this issue, because we are following "https://playwright.dev/docs/test-parallel" to serialise tests in a specific order:

test.describe('first test', aFunction) ;
test.describe('second test', bFunction) ;
test.describe('third test', cFunction) ;

@AptecoJoe
Copy link

Any update on whether this is going to be fixed? Seems to have affected a lot of users including myself

@Jabbar2010
Copy link

@pavelfeldman Hi, any update? This issue has already affected many people.

@Jabbar2010
Copy link

Hi, @pavelfeldman because of this, I can't upgrade to the latest version, why must this limit?

@mprykhodko
Copy link

We were able to fix this issue in our framework. Providing the example below, maybe it will help someone.
We coupled all necessary fixtures in one that we imported in our tests and used aliases for different occasions.

adminMenu FIXTURE:

import { test as baseTest } from "@baseTest"; // baseTest fixture has all common navigations and imports->exports test from from "@playwright/test";
import { loginPage, commonFixtures, etc.} from "@baseTest";
...
export type menuFixtures = {
  settingsPage: SettingsPage;
  etc.,
   ......
};

export const settingsPage= async ({ page }, use) => {
    ....
  await use(settingsPage);
};

export const test = baseTest.extend<menuFixtures & commonFixtures>({
  loginPage,
  anyOtherRequiredFixture,
   ....
});

Th above fixture imports all the necessary fixtures and exports them as well.
Tests look like below

IMPORTS

import { test, expect } from "/adminMenu";
import { test as Setup } from "/adminMenu";
import {
  test as settingsTest,
  expect as settingsExpect,
  settingsPage
} from "/adminMenu";

USAGE:

test.describe("Test....", () => {
        Setup(`Pre-Condition.....`, async ({ loginPage, anyOtherRequiredFixture }) => { }

@nautilux2
Copy link

Any update on this ?

@FelixZilber
Copy link

+1 affected

@Dtesch9
Copy link

Dtesch9 commented Oct 3, 2024

+1

1 similar comment
@ElenaPopova18
Copy link

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment