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

[Question] How to make test() use the same browser instance as the beforeAll. (Ignore test isolation) #14508

Closed
gegoncalves opened this issue May 31, 2022 · 21 comments

Comments

@gegoncalves
Copy link

gegoncalves commented May 31, 2022

UPDATE: Check my comment #14508 (comment)
UPDATE 2: Check my comment #14508 (comment)

I am trying to reuse the authentication example provided at the documentation for multiple .spec files (I know that is not recommended but it is necessary for my case)

Is there any recommendation of POM with fixture that I can do to solve that issue?

I want to avoid rewrite this part multiple times:

test.beforeAll(async ({ browser }) => {
  // Create page once and sign in.
  page = await browser.newPage();
  await page.goto('https://github.com/login');
  await page.fill('input[name="user"]', 'user');
  await page.fill('input[name="password"]', 'password');
  await page.click('text=Sign in');
});
@kryaksy
Copy link

kryaksy commented May 31, 2022

Doesn't this work for you?

// my-test.ts
import { Page, test as base } from '@playwright/test';

interface MyFixtures {
  myPage: Page;
}

export const myTest = base.extend<MyFixtures>({
  myPage: async ({ browser }, use) => {
    // Create page once and sign in.
    const page = await browser.newPage();
    await page.goto('https://github.com/login');
    await page.fill('input[name="user"]', 'user');
    await page.fill('input[name="password"]', 'password');
    await page.click('text=Sign in');
    use(page);
  },
});

// example.spec.ts
myTest.describe('My suite', () => {
  myTest('My test', async ({ myPage }) => {
    await myPage.click('selector');
  });
});

@gegoncalves
Copy link
Author

Unfortunately no. I am using a similar idea to this one

When I go to the next test() it opens another browser instance instead of using the one created in beforeall.

I did what you said in a different file and extended it changing the line 14 and 15 for the { browser }

Doesn't this work for you?

// my-test.ts
import { Page, test as base } from '@playwright/test';

interface MyFixtures {
  myPage: Page;
}

export const myTest = base.extend<MyFixtures>({
  myPage: async ({ browser }, use) => {
    // Create page once and sign in.
    const page = await browser.newPage();
    await page.goto('https://github.com/login');
    await page.fill('input[name="user"]', 'user');
    await page.fill('input[name="password"]', 'password');
    await page.click('text=Sign in');
    use(page);
  },
});

// example.spec.ts
myTest.describe('My suite', () => {
  myTest('My test', async ({ myPage }) => {
    await myPage.click('selector');
  });
});

@kryaksy
Copy link

kryaksy commented May 31, 2022

Yeah, got it now. This might work then. But I don't know if it behaves as you wish for multiple test suites.

// my-test.ts
import { Page, test as base } from '@playwright/test';

interface MyWorkerScopedFixtures {
  myPage: Page;
}

export const myTest = base.extend<{}, MyWorkerScopedFixtures>({
  myPage: [
    async ({ browser }, use) => {
      // Create page once and sign in.
      const page = await browser.newPage();
      await page.goto('https://github.com/login');
      await page.fill('input[name="user"]', 'user');
      await page.fill('input[name="password"]', 'password');
      await page.click('text=Sign in');
      use(page);
    },
    { scope: 'worker' },
  ],
});

// example.spec.ts
myTest.describe('My suite', () => {
  myTest('My test 1', async ({ myPage }) => {
    await myPage.click('selector');
  });

  myTest('My test 2', async ({ myPage }) => {
    await myPage.click('selector');
  });
});

@gegoncalves
Copy link
Author

gegoncalves commented Jun 1, 2022

It always open a new browser instance. I have no idea why.

Let me review my points:

  • I need to sacrifice the isolation and run a number of tests in the same page.
  • beforeAll does not allow page or context usage. So I need to use { browser } fixture to do the login there. And there is the problem:
    • I am using { page } for the other custom fixtures that I have and this is causing to open a new instance of the browser instead of use the one opened on beforeAll
// fixture-list.ts
const test = baseTest.extend<{
    loginContext: LoginContext;
    processPage: ProcessPage;
    cranePage: CranePage;}>({
    loginContext: async ({ browser }, use) => {
        await use(new LoginContext(browser));
    },
    processPage: async ({ page }, use) => {
        await use(new ProcessPage(page));
    },
    cranePage: async ({ page }, use) => {
        await use(new CranePage(page));
    },

// -------------------------------------------------

// test.spec.ts

    test.beforeAll(async ({ loginContext }) => {
      await loginContext.adminUser();
    })

    test('Test 1', async ({ processPage }) => {
      await processPage.navigateToLibrary();
      ...
    });

    test('Test 2', async ({ cranePage }) => {
      await cranePage.navigateToLibrary();
      ...
    });

For any reason, it is always opening a new instance of the browser when it goes to the Test1, test2 ...

loginContext.adminUser(); calls:

export default class LoginContext {

    readonly context: Browser;

    constructor(context: Browser) {
        this.context = context;
    }

    async adminUser(): Promise<void> {
        const page = await this.context.newPage();
        await page.goto('/');
        await page.fill(xxxx, xxxxx);
        await page.fill(xxxxx, xxxx);
        await page.click(xxxxx);

xxxx.navigateToLibrary() calls:

export class WebActions {
    readonly page: Page;

    constructor(page: Page) {
        this.page = page;
    }

    async navigateToLibrary(url: string) {
        await this.page.goto(url);
        // and assert if the page was loaded properly
        ...
    }

Do you think that is something related for one being Page type and the other being Browser? Sorry for the stupid question, I am struggling a lot.

@kryaksy

@gegoncalves gegoncalves changed the title [Question] How to avoid repetition of same steps in a beforeAll hook for different spec files [Question] How to make test() use the same browser instance as the beforeAll. (Ignore test isolation) Jun 1, 2022
@aslushnikov
Copy link
Collaborator

@gegoncalves Check out this documentation entry on a single page re-use: https://playwright.dev/docs/test-retries#reuse-single-page-between-tests

Would it help you?

@gegoncalves
Copy link
Author

gegoncalves commented Jun 1, 2022

@gegoncalves Check out this documentation entry on a single page re-use: https://playwright.dev/docs/test-retries#reuse-single-page-between-tests

Would it help you?

Thanks for the reply @aslushnikov . I tried to do that way, the problem is: I am using POM and fixtures. I don't know how to make all the pages to use the same { browser } context.

the way I implemented it is going to 2 different browser instances:

  • the login from beforeAll
  • another when it goes to the test() itself

I try to explain a little bit more here #14508 (comment)

@aslushnikov
Copy link
Collaborator

the way I implemented it is going to 2 different browser instances:

@gegoncalves yes, this way won't work. The only way forward for you is to re-use the single page like we explain in the documentation and to not use the page fixture in your tests.

I tried to do that way, the problem is: I am using POM and fixtures. I don't know how to make all the pages to use the same { browser } context.

Since you re-use the same page, it'll be the context as well. Can you elaborate what's your problem?

@gegoncalves
Copy link
Author

@gegoncalves yes, this way won't work. The only way forward for you is to re-use the single page like we explain in the documentation and to not use the page fixture in your tests.

Since you re-use the same page, it'll be the context as well. Can you elaborate what's your problem?

Now I got it. So my problem is exactly that: using page

My basic methods all are configured based on Page

//common-actions.ts

export class generalActions{
    readonly page: Page;

    constructor(page: Page) {
        this.page = page;
    }

    async navigateToURL(url: string) {
        await this.page.goto(url);
    }

    async waitForElementAttached(locator: string): Promise<void> {
        await this.page.locator(locator).waitFor();
    }

    async clickElement(locator: string): Promise<void> {
        await this.waitForElementAttached(locator);
        await this.page.locator(locator).click();
    }

So, when I use the beforeAll with the browser it creates one browser instance and because my test() always use methods from the common-actions.ts that have Page, it opens a new browser instance.

Do you recommend any workaround @aslushnikov ?

@aslushnikov
Copy link
Collaborator

@gegoncalves the workaround would be to use the 'single-page re-use pattern' and create POMs next to it. Like this:

import { test, Page } from '@playwright/test';

test.describe.configure({ mode: 'serial' });

let page: Page;
let actions: generalActions;

test.beforeAll(async ({ browser }) => {
  page = await browser.newPage();
  actions = new generalActions(page);
});

test.afterAll(async () => {
  await page.close();
});

test('runs first', async () => {
  await actions.navigateToURL('https://playwright.dev/');
});

test('runs second', async () => {
  await actions.clickElement('text=Get Started');
});

@gegoncalves
Copy link
Author

@gegoncalves the workaround would be to use the 'single-page re-use pattern' and create POMs next to it. Like this:

I got your point. We are using this way:

  • the class generalActions is being called for each page file that I have (the webapp is massive with a lot of modules).
  • Each page file has specific methods for the page
  • After, the class of each page file are extended to a common custom fixture file
  • In the .spec.ts files we just use the necessary custom fixtures

generalActions -> page file -> fixture -> test.spec.ts

Ex:

// fixtures-list.ts

import { test as baseTest } from "@playwright/test";
import ProcessPage from "@pages/ProcessPage";
import CranePage from "@pages/CranePage";
import LoginContext from "@pages/login-context";

const test = baseTest.extend<{
    loginContext: LoginContext;
    processPage: ProcessPage;
    cranePage: CranePage;}>({
    loginContext: async ({ browser }, use) => {
        await use(new LoginContext(browser));
    },
    processPage: async ({ page }, use) => {
        await use(new ProcessPage(page));
    },
    cranePage: async ({ page }, use) => {
        await use(new CranePage(page));
    },

//test.spec.ts

    test.beforeAll(async ({ loginContext }) => {
      await loginContext.adminUser();
    })

    test('check nav', async ({ processPage }) => {
      await processPage.navigateToLibrary();
      const modal = await processPage.addProcess();
      expect(modal).toBeVisible();

Do you have any hint for this scenario @aslushnikov ?

Thank you for helping me with this and sorry for disturb

@aslushnikov
Copy link
Collaborator

aslushnikov commented Jun 2, 2022

@gegoncalves i see what you do, but it won't work.

It might be that I'm missing something: why don't you want to not use fixtures for your POM objects and instead have a global variable, like it's shown here?

@gegoncalves
Copy link
Author

I don't know if I understood your question @aslushnikov correctly

So you are asking to use a global variable instead of the custom fixtures? I was thinking to use the custom fixtures due to the fact of being easier to just call the related pages through fixtures per test case than create an instance of each used POM

The generalActions class will be only the basic method we have. Ex:

async clickElement(locator: string): Promise<void> {
        await this.page.locator(locator).waitFor();
        await this.page.locator(locator).click();
    }

this method we use inside of each PO we have. Ex:

// process-page.ts

  ...
  async addProcess(): Promise<Locator> {
        await generalActions.clickElement(list_locator);
        await generalActions.clickElement(process_locator);
        return this.getModal();
    };
   ...

    async getModal(): Promise<Locator> {
        await generalActions.waitForElementAttached(tab_locator);
        return generalActions.page.locator(MODAL_DIV);
    };

And after we have the fixture creation for each page that I shown here

Should I not use the custom fixture and just do every time a instance of each PO per test set file?

Probably I am doing something stupid without noticing 😅

For real, thanks for giving your time to help me with that

@aslushnikov
Copy link
Collaborator

aslushnikov commented Jun 2, 2022

Should I not use the custom fixture and just do every time a instance of each PO per test set file?

@gegoncalves Yeah, I'd do it without fixtures! It should work reliably in case of single-page re-use.

@gegoncalves
Copy link
Author

gegoncalves commented Jun 2, 2022

I got it.

I was thinking here: It will be nice if there was an option in the future to "turn off" a test() isolation using something in a beforeAll which allowed the usage of page and context fixtures.
I don't know if it make sense for you @aslushnikov
That way would be easier to have a specific .spec.ts file that is not isolated between test()

Maybe you are wondering why we are doing that. The reason is: we are trying to make a TC that use the same login instance to different steps (without the need of reuse signed in state / we just need the state for one TC and not the whole project), it is readable as test steps and generate reports that are easier for the devs to find the issue.

We tried to use as test.step() however we found 3 issues:

  • when one test.step() fails there is no way to keep running the other steps
  • When one test.step() fails the html report does not show the step as failed (maybe it is a bug?)
  • The cli logs and json files are also not good when using test.step(). It requires a lot of time to find the specific cause

Do you think that is a possibility to improvements/fixes for the test.step @aslushnikov ?

image

@aslushnikov
Copy link
Collaborator

When one test.step() fails the html report does not show the step as failed (maybe it is a bug?)

This does look like a bug; but would you mind filing a separate issue with a minimal repro for that?

@gegoncalves
Copy link
Author

gegoncalves commented Jun 2, 2022

When one test.step() fails the html report does not show the step as failed (maybe it is a bug?)

This does look like a bug; but would you mind filing a separate issue with a minimal repro for that?

Yeap, I am gonna do it, no problem at all @aslushnikov .

Regarding the test.step() ending the run when it founds a problem, is it possible to set somehow to keep going to the next steps?

@aslushnikov
Copy link
Collaborator

Regarding the test.step() ending the run when it founds a problem, is it possible to set somehow to keep going to the next steps?

Folding this into #14584!

@msmith1114
Copy link

I got it.

I was thinking here: It will be nice if there was an option in the future to "turn off" a test() isolation using something in a beforeAll which allowed the usage of page and context fixtures. I don't know if it make sense for you @aslushnikov That way would be easier to have a specific .spec.ts file that is not isolated between test()

Maybe you are wondering why we are doing that. The reason is: we are trying to make a TC that use the same login instance to different steps (without the need of reuse signed in state / we just need the state for one TC and not the whole project), it is readable as test steps and generate reports that are easier for the devs to find the issue.

We tried to use as test.step() however we found 3 issues:

  • when one test.step() fails there is no way to keep running the other steps
  • When one test.step() fails the html report does not show the step as failed (maybe it is a bug?)
  • The cli logs and json files are also not good when using test.step(). It requires a lot of time to find the specific cause

Do you think that is a possibility to improvements/fixes for the test.step @aslushnikov ?

image

@gegoncalves
This is a bit old, but I am curious how you ended up implementing it to work the way you were describing? (Since I also use a similar setup where I have "generalActions" page that gets included into my page fixtures.

@gegoncalves
Copy link
Author

gegoncalves commented Jun 1, 2023

Hi @msmith1114

I am not sure if I got your question:
Are you mention about the fact that the step did not show as failed? If this is the question I found what was the issue.
If you look at the error box and line 30 it is :expect(active).toHaveAttribute... should have await at the beginning

If you have a await it gonna fail there but will not be able to continue

@Saif600
Copy link

Saif600 commented Mar 20, 2024

Hi @gegoncalves ,
Thank you for stating the single-browser instance issue on a page. I am also facing the issue. I also have the generalAction class which is used across different pages. Could you please share the way to resolve the issue? Did you just skip making of custom fixture? TIA

@SatishPolisetty
Copy link

SatishPolisetty commented Apr 1, 2024

@aslushnikov On the similar front,
How to write my second spec file to use the same browser session created in spec file1? Actually my need is, I want to have one browser launched and all my tests across all the spec files should run on the same broswer.

Any luck with your attemts @gegoncalves

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants