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

fix(browser): avoid safaridriver collision #4863

Merged
merged 1 commit into from
Jan 4, 2024

Commits on Jan 3, 2024

  1. fix(browser): avoid safaridriver collision

    Removes code from node/providers/webdriver.ts causing the following
    error when running `vitest --browser.name=safari` on macOS:
    
      Error: There is already a Safaridriver instance running on port
      undefined!
    
    safaridriver.start() maintains a static reference to a safaridriver
    instance, and will fail with this error if start() is called again
    before stop().
    
    The original node/providers/webdriver.ts calls safaridriver.start()
    directly, and then immediately imports and calls webdriverio.remote().
    This call also eventually calls safaridriver.start(), producing the
    error.
    
    The fix is to remove the direct safaridriver access, as webdriverio
    seems to manage safaridriver successfully now.
    
    ---
    
    Testing:
    
    Since Safari can't run in headless mode, I couldn't think of a way to
    add a test that would run in CI. However, I added a new
    test:safaridriver target intended to be run manually as needed, which
    runs all the existing test/browser tests under Safari.
    
    ---
    
    One extra observation:
    
    Command line options like --browser.headless=false are _not_ converted
    to boolean values; they remain string values.
    
    This is why the `--browser.headless` argument is now pushed onto the
    extracted argv[], after I had originally tried updating the existing
    argument string to:
    
      `--browser.headless=${browser !== 'safari'}`
    
    When I updated the `pnpm:safaridriver` target with `PROVIDER=playwright`
    to see what would happen (even though this is only a webdriver change),
    it produced this error:
    
      Error: browserType.launch: headless: expected boolean, got string
    	 ❯ PlaywrightBrowserProvider.openBrowserPage
         ../../packages/browser/dist/providers.js:22:52
           20|     const options = this.ctx.config.browser;
           21|     const playwright = await import('playwright');
    			 22|     const browser = await playwright[this.browser].launch({
             |                                                    ^
           23|       ...this.options?.launch,
           24|       headless: options.headless
    
    You can actually see this by running:
    
      npx vitest --run --browser.name=webkit --browser.provider=playwright
        --browser.headless=false
    
    This seems like a bug worth filing, but not addressing in this change.
    mbland committed Jan 3, 2024
    Configuration menu
    Copy the full SHA
    86de156 View commit details
    Browse the repository at this point in the history