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

Setting up an environment with persistentContext #188

Closed
karthikiyengar opened this issue Jul 2, 2020 · 12 comments · Fixed by #193
Closed

Setting up an environment with persistentContext #188

karthikiyengar opened this issue Jul 2, 2020 · 12 comments · Fixed by #193
Labels
enhancement New feature or request

Comments

@karthikiyengar
Copy link
Contributor

karthikiyengar commented Jul 2, 2020

Is your feature request related to a problem? Please describe.
In order to test Chrome Extensions (and other persistent use cases), it would be nice if chrome can be launched in persistent mode. Explicitly passing a userDataDir to launch results in
userDataDir option is not supported in `browserType.launch`. Use `browserType.launchPersistentContext` instead

Describe the solution you'd like

Some way or flag to override https://github.com/playwright-community/jest-playwright/blob/master/src/PlaywrightEnvironment.ts#L32 and use launchPersistentContext instead of launch. It can be as simple as the following, although it must be noted that we will no longer comply with playwright's launch options.

if (launchOptions.userDataDir != null) {
  return playwrightInstance.launch(launchOptions.userDataDir, options)
}

An alternative would be to create a new option along the lines of persistentDataDir.

This would allow us to setup a custom environment for extensions like so:

class CustomEnvironment extends PlaywrightEnvironment {
  async setup() {
    await super.setup()
    let popupPageUrl

    const backgroundPage = await this.global.browser.waitForEvent('backgroundpage') // this is only available via chromium.launchPersistentContext
    const extensionId = backgroundPage.url().split('/')[2]
    popupPageUrl = `chrome-extension://${extensionId}/popup.html#`

    this.global.page.gotoPopup = function (url, options = {}) {
      const paddedUrl = !url.startsWith('/') ? `/${url}` : url
      return page.goto(popupPageUrl + paddedUrl, options)
    }
  }

  async teardown() {
    // Your teardown
    await super.teardown()
  }
}
@karthikiyengar
Copy link
Contributor Author

I'm pretty sure there's a more DRY way to do this, but I've opened #192 to kickstart a discussion. Note that the browser global will be undefined in case we use the persistentDataDir variable.

Additionally, it looks like there's a playwright bug that needs to be addressed around this behaviour. I've opened an issue to track it here: microsoft/playwright#2828

@karthikiyengar
Copy link
Contributor Author

karthikiyengar commented Jul 6, 2020

@mmarkelov - I'm not sure that your PR actually solves the issue. I'm running into the same issue as the one I described on microsoft/playwright#2828 when there are two test files. I'm running the latest beta with the following config:

module.exports = {
  launchOptions: {
    headless: false,
    args: [
      `--disable-extensions-except=${extensionDistDir}`,
      `--load-extension=${extensionDistDir}`,
    ],
  },
  launchType: "PERSISTENT",
  userDataDir: extensionDataDir,
}

The path of least resistance to solve this (per comment here) would be to add a random nonce to the userDataDir.

@mmarkelov mmarkelov reopened this Jul 8, 2020
@mmarkelov
Copy link
Member

@karthikiyengar I can add Date.now() to userDataDir for each new browser. But just not sure that it will help

@karthikiyengar
Copy link
Contributor Author

@mmarkelov - Date.now() is probably not the best idea, it should probably be a function of the test suite filename/name to avoid buildup of way too many profiles.

I'm running it with Date.now() for now on local and it works perfectly fine. For the rationale behind this, please refer to puppeteer/puppeteer#3440 (comment) and microsoft/playwright#2828 (comment)

The only way to spawn a new context per test suite in our case would be to have a different folder AFAIK, since launchPersistentContext returns a BrowserContext (and not a Browser).

@mmarkelov
Copy link
Member

@karthikiyengar I just don't have any experience with launchPersistentContext, so I can dynamically change userDataDir to this: config.userDataDir/currentFilename. Will it be nicer?

@karthikiyengar
Copy link
Contributor Author

@mmarkelov - Yes, that will get the job done.

I will add that I'm not entirely sure whether the approach we're taking with this is correct though, and sadly I lack the time to dig deeper. I've moved to puppeteer for now. For the record, jest-puppeteerspawns tabs for individual test files rather than creating a new profile when using persistent context.

Also, another difference in behaviour I've noticed is that jest-playwright prefers an incognito session by default whereas jest-puppeteer doesn't.

@gilles-yvetot
Copy link

I'm pretty sure there's a more DRY way to do this, but I've opened #192 to kickstart a discussion. Note that the browser global will be undefined in case we use the persistentDataDir variable.

Additionally, it looks like there's a playwright bug that needs to be addressed around this behaviour. I've opened an issue to track it here: microsoft/playwright#2828

@karthikiyengar, I have the exact same scenario than @mmarkelov where I need to test an extension and it looks like it has to be done from a persistent context. I am hitting the error your mentioned where the global browser is null. Any idea why? Anything you would recommend to fix/address that?

@krashdifferent
Copy link

Just adding another use case where we have a similar issue with persistent context and multiple test files failing with jest-playwright. In this case we are using the persistent context to handle Multifactor Authentication. Following the playwright docs, we manually run a separate script before the tests to setup the MFA login state then run the tests using that profile. The main difference I see from the above is in how the existing profile needs to be generated.

Outside of manually handing playwright setup and jest integration, are there other workarounds people have found for this?

@mmarkelov
Copy link
Member

@karthikiyengar I don't have much experience with MFA testing. Could you please provide some test examples? I tried to look through and help you if I can

@krashdifferent
Copy link

@mmarkelov We are not testing the MFA itself so much as we need to get though the authentication in order to test the rest of our app. Since MFA requires a separate passcode sent through a phone on each login, we cannot practically automate the login itself.

Instead, before running the automated tests, we launch the browser with a persistent context and login manually. The login state is saved in the persistent data of the browser. We then launch the automated tests pointing to this same persistent data. Since the login is already saved, this allows us to bypass the need to login again when the automated tests are running.

The problem is every browser context needs to point to this same persistent profile or at least a copy of it in order to bypass the login. When running the tests in series with jest-playwright, the first test file completes without issue, but the next file fails to open a new browser as playwright fails to attach to the process. As far as I can tell this is the same issue as above. Just running a setup using the PERSISTENT type and multiple test files should replicate the basic issue independent of MFA.

I've come up with two work arounds if I am not using jest-playwright and instead handle the jest integration myself. This first is simply to make sure the browser context fully closes between each test suite. Basically creating the browser context in the beforeAll and await browser.close() in the afterAll of each test file. The second and more powerful way is to have each test file create a temporary copy of the persistent data and launch the context using the copy. This allows the tests to run in parallel and prevents the tests from inadvertently impacting the persistent state of another test down stream.

I haven't been able to figure out a way to replicate either of the above methods using jest-playwright. I do not seem to be able to reset or fully close the browser context between test files when running in series and with the global context and only one userDataDir, I couldn't think of a way to mimic the copy method.

I don't know the best approach but I'd love to switch to jest-playwright so I don't also have to separately deal with coverage or launching the server. Any insights are greatly appreciated!

@mmarkelov
Copy link
Member

@karthikiyengar I just not sure about nice way of implementing this kind of tests. So I just build some small examples.

// jest-playwright.config.js
module.exports = {
    launchType: "PERSISTENT",
    userDataDir: "test",
}

// tests
beforeAll(async () => {
    await page.goto('https://jsfiddle.net/')
    const sign = await page.$eval('#actions > ul > li:nth-child(2) > a', (el) => el.innerHTML)
    if (sign === 'Sign in') {
        await page.goto('https://jsfiddle.net/user/login/')
        await page.fill('#id_username', 'username');
        await page.fill('#id_password', 'password');

        await page.click('.buttonCont');
    }
})

test('title', async () => {
    const title = await page.title()
    expect(title).toBe('JSFiddle - Code Playground')
})

test('should display nick', async () => {
    await page.goto('https://jsfiddle.net/user/fiddles/all/')
    const nick = await page.$eval('#user-top-bar strong', (el) => el.innerHTML)
    expect(nick).toContain('username')
})

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

@mxschmitt
Copy link
Member

Closed because of inactivity. Feel free to reopen if its still an issue or create a new one.

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

Successfully merging a pull request may close this issue.

5 participants