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

Use PLAYWRIGHT_DRIVER_PATH environment variable as driver cache directory #485

Merged

Conversation

GuyGoldenberg
Copy link
Contributor

@GuyGoldenberg GuyGoldenberg commented Sep 12, 2024

Playwright uses PLAYWRIGHT_BROWSERS_PATH to define the cache directory used to install the browsers.
See playwright implementation - link

  const envDefined = getFromENV('PLAYWRIGHT_BROWSERS_PATH');
  if (envDefined === '0') {
    result = path.join(__dirname, '..', '..', '..', '.local-browsers');
  } else if (envDefined) {
    result = envDefined;
  } else {
    let cacheDirectory: string;
    if (process.platform === 'linux')
      cacheDirectory = process.env.XDG_CACHE_HOME || path.join(os.homedir(), '.cache');
    else if (process.platform === 'darwin')
      cacheDirectory = path.join(os.homedir(), 'Library', 'Caches');
    else if (process.platform === 'win32')
      cacheDirectory = process.env.LOCALAPPDATA || path.join(os.homedir(), 'AppData', 'Local');
    else
      throw new Error('Unsupported platform: ' + process.platform);
    result = path.join(cacheDirectory, 'ms-playwright');
  }

@canstand
Copy link
Collaborator

@GuyGoldenberg The Playwright driver directory is used for clients only. And PLAYWRIGHT_BROWSERS_PATH has a significantly different intent. That's why I prefer to add the PLAYWRIGHT_DRIVER_PATH environment variable.

run.go Outdated
@@ -37,6 +37,9 @@ type PlaywrightDriver struct {

func NewDriver(options *RunOptions) (*PlaywrightDriver, error) {
baseDriverDirectory := options.DriverDirectory
if baseDriverDirectory == "" {
baseDriverDirectory = os.Getenv("PLAYWRIGHT_DRIVER_PATH")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I'd probably put it into into the first line of getDefaultCacheDirectory, then the caller side is simpler. Either the user given value or our internal logic.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it in transformRunOptions and added some comments. getDefaultCacheDirectory still maintains its single purpose.

Users can now understand the usage through the type comments of RunOptions.

@mxschmitt mxschmitt changed the title Use PLAYWRIGHT_BROWSERS_PATH environment variable as driver cache directory Use PLAYWRIGHT_DRIVER_PATH environment variable as driver cache directory Sep 16, 2024
@canstand canstand merged commit 8ce48a3 into playwright-community:main Sep 19, 2024
22 checks passed
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

Successfully merging this pull request may close these issues.

3 participants