fix: Detect user-configured browsers (#23446)

* fix: Detect user-configured browsers

* move user browser lookup into BrowserDataSource

* refactor out common browser dedupe logic

* simplify allBrowsers

* resolve non-machine browsers

* pr feedback

* added tests

* fix test

* longer timeout

Co-authored-by: Lachlan Miller <lachlan.miller.1990@outlook.com>
Co-authored-by: Emily Rohrbough <emilyrohrbough@users.noreply.github.com>
Co-authored-by: Zachary Williams <ZachJW34@gmail.com>

npm/cypress-schematic/src/ct.spec.ts

@@ -38,7 +38,7 @@ const cypressSchematicPackagePath = path.join(__dirname, '..')
 const ANGULAR_PROJECTS: ProjectFixtureDir[] = ['angular-13', 'angular-14']
 
 describe('ng add @cypress/schematic / e2e and ct', function () {
-  this.timeout(1000 * 60 * 4)
+  this.timeout(1000 * 60 * 5)
 
   for (const project of ANGULAR_PROJECTS) {
     it('should install ct files with option and no component specs', async () => {

packages/data-context/src/data/ProjectLifecycleManager.ts

@@ -296,7 +296,7 @@ export class ProjectLifecycleManager {
 
     // lastBrowser is cached per-project.
     const prefs = await this.ctx.project.getProjectPreferences(path.basename(this.projectRoot))
-    const browsers = await this.ctx.browser.machineBrowsers()
+    const browsers = await this.ctx.browser.allBrowsers()
 
     if (!browsers[0]) {
       this.ctx.onError(getError('UNEXPECTED_INTERNAL_ERROR', new Error('No browsers found, cannot set a browser')))

packages/data-context/src/data/coreDataShape.ts

@@ -119,6 +119,7 @@ export interface CoreDataShape {
   cliTestingType: string | null
   activeBrowser: FoundBrowser | null
   machineBrowsers: Promise<FoundBrowser[]> | null
+  allBrowsers: Promise<FoundBrowser[]> | null
   servers: {
     appServer?: Maybe<Server>
     appServerPort?: Maybe<number>
@@ -160,6 +161,7 @@ export function makeCoreData (modeOptions: Partial<AllModeOptions> = {}): CoreDa
     cliBrowser: modeOptions.browser ?? null,
     cliTestingType: modeOptions.testingType ?? null,
     machineBrowsers: null,
+    allBrowsers: null,
     hasInitializedMode: null,
     dashboardGraphQLError: null,
     dev: {

packages/data-context/src/sources/BrowserDataSource.ts

@@ -3,6 +3,7 @@ import os from 'os'
 import execa from 'execa'
 
 import type { DataContext } from '..'
+import _ from 'lodash'
 
 let isPowerShellAvailable: undefined | boolean
 let powerShellPromise: Promise<void> | undefined
@@ -22,6 +23,14 @@ if (os.platform() === 'win32') {
 
 const platform = os.platform()
 
+function getBrowserKey<T extends {name: string, version: string | number}> (browser: T) {
+  return `${browser.name}-${browser.version}`
+}
+
+export function removeDuplicateBrowsers (browsers: FoundBrowser[]) {
+  return _.uniqBy(browsers, getBrowserKey)
+}
+
 export interface BrowserApiShape {
   close(): Promise<any>
   ensureAndGetByNameOrPath(nameOrPath: string): Promise<FoundBrowser>
@@ -33,29 +42,61 @@ export interface BrowserApiShape {
 export class BrowserDataSource {
   constructor (private ctx: DataContext) {}
 
+  /**
+   * Gets the browsers from the machine and the project config
+   */
+  async allBrowsers () {
+    if (this.ctx.coreData.allBrowsers) {
+      return this.ctx.coreData.allBrowsers
+    }
+
+    const p = await this.ctx.project.getConfig()
+    const machineBrowsers = await this.machineBrowsers()
+
+    if (!p.browsers) {
+      this.ctx.coreData.allBrowsers = Promise.resolve(machineBrowsers)
+
+      return this.ctx.coreData.allBrowsers
+    }
+
+    const userBrowsers = p.browsers.reduce<FoundBrowser[]>((acc, b) => {
+      if (_.includes(_.map(machineBrowsers, getBrowserKey), getBrowserKey(b))) return acc
+
+      return [...acc, {
+        ...b,
+        majorVersion: String(b.majorVersion),
+        custom: true,
+      }]
+    }, [])
+
+    this.ctx.coreData.allBrowsers = Promise.resolve(_.concat(machineBrowsers, userBrowsers))
+
+    return this.ctx.coreData.allBrowsers
+  }
+
   /**
    * Gets the browsers from the machine, caching the Promise on the coreData
    * so we only look them up once
    */
   machineBrowsers () {
-    if (!this.ctx.coreData.machineBrowsers) {
-      const p = this.ctx._apis.browserApi.getBrowsers()
+    if (this.ctx.coreData.machineBrowsers) {
+      return this.ctx.coreData.machineBrowsers
+    }
 
-      this.ctx.coreData.machineBrowsers = p.then(async (browsers) => {
-        if (!browsers[0]) throw new Error('no browsers found in machineBrowsers')
+    const p = this.ctx._apis.browserApi.getBrowsers()
 
-        return browsers
-      }).catch((e) => {
-        this.ctx.update((coreData) => {
-          coreData.machineBrowsers = null
-          coreData.diagnostics.error = e
-        })
+    return this.ctx.coreData.machineBrowsers = p.then(async (browsers) => {
+      if (!browsers[0]) throw new Error('no browsers found in machineBrowsers')
 
-        throw e
+      return browsers
+    }).catch((e) => {
+      this.ctx.update((coreData) => {
+        coreData.machineBrowsers = null
+        coreData.diagnostics.error = e
       })
-    }
 
-    return this.ctx.coreData.machineBrowsers
+      throw e
+    })
   }
 
   idForBrowser (obj: FoundBrowser) {

packages/data-context/test/fixtures/browsers.ts

@@ -0,0 +1,19 @@
+import { FoundBrowser } from "@packages/types";
+
+export const foundBrowserChrome: FoundBrowser = {
+  name: 'chrome',
+  family: 'chromium',
+  channel: 'stable',
+  displayName: 'Chrome',
+  path: '/usr/bin/chrome',
+  version: '100.0.0'
+} as const
+
+export const userBrowser: Cypress.Browser = {
+  ...foundBrowserChrome,
+  name: 'User Custom Chromium Build',
+  isHeaded: true,
+  isHeadless: false,
+  family: 'chromium',
+  majorVersion: '100',
+}

packages/data-context/test/unit/data/ProjectLifecycleManager.spec.ts

@@ -2,6 +2,7 @@ import { expect } from 'chai'
 import type { DataContext } from '../../../src'
 import { createTestDataContext } from '../helper'
 import sinon from 'sinon'
+import { FullConfig } from '@packages/types'
 
 const browsers = [
   { name: 'electron', family: 'chromium', channel: 'stable', displayName: 'Electron' },
@@ -28,9 +29,15 @@ function createDataContext (modeOptions?: Parameters<typeof createTestDataContex
   return context
 }
 
+const fullConfig: FullConfig = {
+  resolved: {},
+  browsers: [],
+}
+
 describe('ProjectLifecycleManager', () => {
   beforeEach(() => {
     ctx = createDataContext()
+    sinon.stub(ctx.lifecycleManager, 'getFullInitialConfig').resolves(fullConfig)
   })
 
   context('#setInitialActiveBrowser', () => {

packages/data-context/test/unit/sources/BrowserDataSource.spec.ts

@@ -0,0 +1,76 @@
+import chai from 'chai'
+import sinon from 'sinon'
+import sinonChai from 'sinon-chai'
+import { FullConfig } from '@packages/types'
+import { createTestDataContext } from '../helper'
+import { userBrowser, foundBrowserChrome } from '../../fixtures/browsers'
+
+chai.use(sinonChai)
+const { expect } = chai
+
+describe('BrowserDataSource', () => {
+  describe('#allBrowsers', () => {
+    it('returns machine browser if no user custom browsers resolved in config', async () => {
+      const fullConfig: FullConfig = {
+        resolved: {},
+        browsers: [],
+      }
+
+      const ctx = createTestDataContext('run')
+
+      sinon.stub(ctx.lifecycleManager, 'getFullInitialConfig').resolves(fullConfig)
+      ctx.coreData.machineBrowsers = Promise.resolve([foundBrowserChrome])
+
+      const result = await ctx.browser.allBrowsers()
+
+      expect(result).to.eql([foundBrowserChrome])
+    })
+
+    it('populates coreData.allBrowsers is not populated', async () => {
+      const fullConfig: FullConfig = {
+        resolved: {},
+        browsers: [userBrowser],
+      }
+
+      const ctx = createTestDataContext('run')
+
+      sinon.stub(ctx.lifecycleManager, 'getFullInitialConfig').resolves(fullConfig)
+      ctx.coreData.machineBrowsers = Promise.resolve([foundBrowserChrome])
+
+      const result = await ctx.browser.allBrowsers()
+
+      expect(result.length).to.eq(2)
+      expect(result[1].custom).to.be.true
+    })
+
+    it('does not add user custom browser if name and version matchnes a machine browser', async () => {
+      const browser = { ...userBrowser, name: 'aaa', version: '100' }
+      const machineBrowser = { ...foundBrowserChrome, name: 'aaa', version: '100' }
+
+      const fullConfig: FullConfig = {
+        resolved: {},
+        browsers: [browser],
+      }
+
+      const ctx = createTestDataContext('run')
+
+      sinon.stub(ctx.lifecycleManager, 'getFullInitialConfig').resolves(fullConfig)
+      ctx.coreData.machineBrowsers = Promise.resolve([machineBrowser])
+
+      const result = await ctx.browser.allBrowsers()
+
+      expect(result).to.eql([machineBrowser])
+    })
+
+    it('returns coreData.allBrowsers if populated', async () => {
+      const allBrowsers = [foundBrowserChrome]
+      const ctx = createTestDataContext('run')
+
+      ctx.coreData.allBrowsers = Promise.resolve(allBrowsers)
+
+      const result = await ctx.browser.allBrowsers()
+
+      expect(result).to.eql(allBrowsers)
+    })
+  })
+})

packages/launcher/lib/detect.ts

@@ -1,6 +1,7 @@
 import Bluebird from 'bluebird'
 import _, { compact, extend, find } from 'lodash'
 import os from 'os'
+import { removeDuplicateBrowsers } from '@packages/data-context/src/sources/BrowserDataSource'
 import { browsers, validateMinVersion } from './browsers'
 import * as darwinHelper from './darwin'
 import { notDetectedAtPathErr } from './errors'
@@ -150,11 +151,6 @@ export const detect = (goalBrowsers?: Browser[]): Bluebird<FoundBrowser[]> => {
     goalBrowsers = browsers
   }
 
-  const removeDuplicates = (val) => {
-    return _.uniqBy(val, (browser: FoundBrowser) => {
-      return `${browser.name}-${browser.version}`
-    })
-  }
   const compactFalse = (browsers: any[]) => {
     return compact(browsers) as FoundBrowser[]
   }
@@ -164,7 +160,7 @@ export const detect = (goalBrowsers?: Browser[]): Bluebird<FoundBrowser[]> => {
   return Bluebird.mapSeries(goalBrowsers, checkBrowser)
   .then((val) => _.flatten(val))
   .then(compactFalse)
-  .then(removeDuplicates)
+  .then(removeDuplicateBrowsers)
 }
 
 export const detectByPath = (

packages/server/lib/makeDataContext.ts

@@ -47,7 +47,7 @@ export function makeDataContext (options: MakeDataContextOptions): DataContext {
       close: browsers.close,
       getBrowsers,
       async ensureAndGetByNameOrPath (nameOrPath: string) {
-        const browsers = await ctx.browser.machineBrowsers()
+        const browsers = await ctx.browser.allBrowsers()
 
         return await ensureAndGetByNameOrPath(nameOrPath, false, browsers)
       },