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

Support different browsers when using multiple projects #71

Closed
BTMorton opened this issue Apr 5, 2023 · 8 comments
Closed

Support different browsers when using multiple projects #71

BTMorton opened this issue Apr 5, 2023 · 8 comments
Labels
enhancement New feature or request

Comments

@BTMorton
Copy link
Contributor

BTMorton commented Apr 5, 2023

Is Your Feature Request related to a Problem? Please Describe

I have an Angular project with three sub-projects that each have their own karma config file and currently use different custom launchers: two use chrome, one uses electron. As the electron project won't run in the chrome environment, I need to run them all in electron. This means I need to have the karma-electron plugin imported into the other two project's karma config files.

Describe the Solution You'd Like

It would be great if instead of using a global default custom launcher the launcher configuration from the karma config could be respected, assuming no custom browser or launcher is specified in the extension settings. There already appears to be some config merging happening for each project as I need to define the plugin in each of them. Could the custom launchers list be maintained, or used as the basis for the default launcher if not being overriden by settings?

Describe Alternatives You've Considered

An alternative approach I considered was being able to specify a custom launcher configuration for each project in the extension settings. This could be added to the existing customLauncher property by supporting a default and allowing project specific overrides, something like:

"karmaTestExplorer.customLauncher": {
  "base": "Chrome",
  "key": "value",
  "project3": {
    "base": "Electron",
    "key": "value"
  }
}

Or by using a new projectLauncher key in addition to the existing configurations:

"karmaTestExplorer.browser": "Chrome",
"karmaTestExplorer.projectLauncher": {
  "project3": {
    "base": "Electron",
    "key": "value"
  }
}

I'd even be happy having to specify all three projects directly, but figured the hybrid approach would be easy enough to implement.

Another alternative could be to allow defining additional plugin imports, so that modifications don't need to be made to each project file, but that's a less nice approach as it still results in different environments for the test runner vs the CLI.

Additional Context

I had some issues getting the electron browser environment working, but I've added context to that in #52 as it's a separate issue.

@BTMorton BTMorton added the enhancement New feature or request label Apr 5, 2023
@lucono
Copy link
Owner

lucono commented Apr 6, 2023

It would be great if instead of using a global default custom launcher the launcher configuration from the karma config could be respected, assuming no custom browser or launcher is specified in the extension settings.

The reason the extension overrides the browser setting of the karma config is to configure it for running and debugging tests in a way that reliably interfaces with Karma Test Explorer. The karmaTestExplorer.browser setting can also then be used for specifying other browsers of choice, but leaves you responsible for also providing a corresponding value for the karmaTestExplorer.debuggerConfig or karmaTestExplorer.debuggerConfigName setting that must ensure that test debugging will work with the chosen browser.

One potential improvement could perhaps be having some browser-aware logic for the most popular browsers while falling back to Chrome in other cases.

@BTMorton
Copy link
Contributor Author

BTMorton commented Apr 6, 2023

That makes sense, and gives me some new config keys to play around with that I wasn't aware of before.

I'm not super tied to the "infer everything" approach if doing that browser-aware logic would be a lot of extra work. Potentially one of the other alternatives where it's still manually defined but can be done per-project would be a simpler solution, and arguably more flexible. I don't mind having to do a bunch more config :D

@lucono
Copy link
Owner

lucono commented Apr 14, 2023

Since projects usually already have a working Karma/Angular setup, and the extension has access to all their relevant configuration (including the Karma config), I would favor where possible, an attempt to maintain fidelity with the project for test execution. So basically relying on that information as a first source for executing the tests, rather than requiring duplicate configuration in the extension.

In this case, I think it would be reasonable to support the few top browsers that cover the majority of users, and fall back to the extension's current default behavior of chrome, or explicit configuration in the extension, which when provided would take precedence over the other behaviors.

So basically:

  1. Make a best case attempt to discover project's configured testing browser, and if browser is one of the top browsers (Chrome, Edge, Firefox) with well known stable configuration parameters, maintain the project's testing configuration and setup the extension to interface accordingly with the browser for testing and debugging
  2. If browser is not one of the top browsers , or if browser could not be discovered:
    1. If extension config has explicit configuration for testing with the project's browser, use those settings for testing and debugging in VS Code
    2. If extension does not have explicit configuration for testing with the browser, use the current default behavior, which is to fall back to using Chrome browser

Items 1 and 2(ii) would be preferred for initial implementation, and 2(i) at some subsequent point if browser support in project-specific configuration is added.

@BTMorton
Copy link
Contributor Author

That sounds like a good solution. For 2(i), would this be something like:

"karmaTestExplorer.browserDefinition": {
  "Electron": {
    "customLauncher": {
      "flags": [
        "--remote-debugging-port=9222",
      ]
    },
    "debuggerConfig": {
      "type": "chrome",
      "request": "attach",
      "port": 9222,
    }
  }
}

where you can configure the launcher options and debugger config to use when that browser is seem in the karma config? Is there a risk that if multiple projects use the same configured browser there could be a debugger port clash? I ran into that issue the other day with multiple projects using the same electron configuration and all trying to use the same remote debugging port.

It did get me thinking that maybe there's some merit in having a more general project-specific settings option that allows overriding any (relevant) config key. Something like:

"karmaTestExplorer.projectSettings": {
  "project1": {
    "showOnlyFocusedTests": true,
    "customLauncher": {...},
    "debuggerName": "Project 1 Debugger",
    "env": {
      "KEY1": "value1"
    }
  },
  "project2": {
    "excludeDisabledTests": true,
    "nonHeadlessModeEnabled": true,
    "debuggerName": "Project 2 Debugger",
    "env": {
      "KEY2": "value2"
    }
  }
}

This could potentially work in partnership with the existing projectWorkspaces key, adding support for additional config tweaks to the execution or display of each project's tests. I can open that a separate issue, if it's something you think would be feasible. Could also potentially serve as a short-term/first-pass solution to this issue while working on your proposed solution.

@lucono
Copy link
Owner

lucono commented Apr 17, 2023

Is there a risk that if multiple projects use the same configured browser there could be a debugger port clash? I ran into that issue the other day with multiple projects using the same electron configuration and all trying to use the same remote debugging port.

Karma Test Explorer has a cross-project PortAcquisitionManager (and per-project client) for exactly this purpose. It manages all port usage by the extension to ensure that there is never a port clash. That includes Karma server ports for any number of projects in a workspace, the extension's interface port with Karma, and the debugging ports for all launched headless or headful testing browsers. The only time the extension does not manage ports is for the debugger port when the user has specified their own custom browser and/or debugger configuration, which is the case you're running into.

It did get me thinking that maybe there's some merit in having a more general project-specific settings option that allows overriding any (relevant) config key ...
This could potentially work in partnership with the existing projectWorkspaces key, adding support for additional config tweaks to the execution or display of each project's tests.

This is also how I imagine support for per-project browser configuration - by extending the existing project-specific configuration capability. Ideally, this would require schema re-use support in VSCode settings, which is currently missing. There's an open feature request on their backlog for it here.

Another challenge with project-specific browser configuration in the extension is that the existing support does not currently understand sub-projects, so it would require first adding general support for sub-project configuration before adding browser configuration specifically at the sub-project level.

If you would like to contribute this support, and the subsequent browser-specific configuration support, I would be willing to review and accept it, but preferably after support has been added for well-known browsers.

I don't know much about Electron, but for well-known browsers support, I imagine that it might be possible to support all Chromium browsers as a group (?) So rather than Chrome, Firefox and Edge, it could just be Firefox and Chromium browsers, with Chromium covering Chrome as well as Electron (?)

@lucono
Copy link
Owner

lucono commented May 27, 2023

@BTMorton were you able to figure out a way forward for #73, or should we close?

@BTMorton
Copy link
Contributor Author

I have some ideas for how to tackle it, but haven't had a chance to work on it again yet

@lucono
Copy link
Owner

lucono commented Jul 16, 2023

@BTMorton I'm closing this for now. Please re-open when able to work on it. Thanks.

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

No branches or pull requests

2 participants