ronantakizawa/codereview-bench huggingface dataset:
Repository: cypress-io/cypress
File: packages/launchpad/cypress/e2e/integration/choose-a-browser.spec.ts (TypeScript)

Code before:
import type { FoundBrowser } from '@packages/types/src'

describe('Choose a Browser Page', () => {
  // Walks through setup pages to get to browser selection
  const stepThroughConfigPages = () => {
    cy.get('h1').should('contain', 'Configuration Files')
    cy.contains('Continue').click()
    cy.get('h1').should('contain', 'Initializing Config...')
    cy.contains('Next Step').click()
  }

  // Mocks server's browser detection with known list
  const setupMockBrowsers = function () {
    cy.withCtx(async (ctx, o) => {
      const mockBrowsers = [{
        channel: 'stable',
        displayName: 'Chrome',
        family: 'chromium',
        name: 'chrome',
        version: '1.2.333.445',
        path: '/test/chrome/path',
        majorVersion: '1',
      }, {
        channel: 'stable',
        displayName: 'Firefox',
        family: 'firefox',
        name: 'firefox',
        path: '/test/firefox/path',
        version: '2.3.444',
        majorVersion: '2',
      }, {
        channel: 'stable',
        displayName: 'Electron',
        family: 'chromium',
        name: 'electron',
        path: '/test/electron/path',
        version: '3.4.555.66',
        majorVersion: '3',
      }, {
        channel: 'stable',

Review comment (suggestion):
```suggestion
      const mockBrowsers: FoundBrowser[] = [{
```

Code after:
describe('Choose a Browser Page', () => {
  // Walks through setup pages to get to browser selection
  const stepThroughConfigPages = () => {
    cy.get('h1').should('contain', 'Configuration Files')
    cy.contains('button', 'Continue').click()
    cy.get('h1').should('contain', 'Initializing Config...')
    cy.contains('button', 'Next Step').click()
  }

  beforeEach(() => {
    cy.scaffoldProject('launchpad')
  })

  describe('System Browsers Detected', () => {
    beforeEach(() => {
      cy.findBrowsers({
        filter: (browser) => {
          return Cypress._.includes(['chrome', 'firefox', 'electron', 'edge'], browser.name) && browser.channel === 'stable'
        },
      })
    })

    it('preselects browser that is provided through the command line', () => {
      cy.openProject('launchpad', ['--e2e', '--browser', 'edge'])

      cy.visitLaunchpad()
      stepThroughConfigPages()

      cy.get('h1').should('contain', 'Choose a Browser')

      cy.findByRole('radio', { name: 'Edge v8', checked: true })
    })

    it('shows warning when launched with --browser name that cannot be matched to found browsers', () => {
      cy.openProject('launchpad', ['--e2e', '--browser', 'doesNotExist'])
      cy.visitLaunchpad()

      stepThroughConfigPages()

      cy.get('h1').should('contain', 'Choose a Browser')
Repository: cypress-io/cypress
File: packages/launchpad/cypress/e2e/integration/choose-a-browser.spec.ts (TypeScript)

Code before:
import type { FoundBrowser } from '@packages/types/src'

describe('Choose a Browser Page', () => {
  // Walks through setup pages to get to browser selection
  const stepThroughConfigPages = () => {
    cy.get('h1').should('contain', 'Configuration Files')
    cy.contains('Continue').click()
    cy.get('h1').should('contain', 'Initializing Config...')
    cy.contains('Next Step').click()
  }

  // Mocks server's browser detection with known list
  const setupMockBrowsers = function () {
    cy.withCtx(async (ctx, o) => {
      const mockBrowsers = [{
        channel: 'stable',
        displayName: 'Chrome',
        family: 'chromium',
        name: 'chrome',
        version: '1.2.333.445',
        path: '/test/chrome/path',
        majorVersion: '1',
      }, {
        channel: 'stable',
        displayName: 'Firefox',
        family: 'firefox',
        name: 'firefox',
        path: '/test/firefox/path',
        version: '2.3.444',
        majorVersion: '2',
      }, {
        channel: 'stable',

Review comment (bug):
Could we update this (and similar locators) to include the element, like `cy.contains('button', 'Continue')`. Ideally the test should fail if that's not a button element, since it would (at least probably) not have the accessibility behaviors that come for free with buttons.

Code after:
describe('Choose a Browser Page', () => {
  // Walks through setup pages to get to browser selection
  const stepThroughConfigPages = () => {
    cy.get('h1').should('contain', 'Configuration Files')
    cy.contains('button', 'Continue').click()
    cy.get('h1').should('contain', 'Initializing Config...')
    cy.contains('button', 'Next Step').click()
  }

  beforeEach(() => {
    cy.scaffoldProject('launchpad')
  })

  describe('System Browsers Detected', () => {
    beforeEach(() => {
      cy.findBrowsers({
        filter: (browser) => {
          return Cypress._.includes(['chrome', 'firefox', 'electron', 'edge'], browser.name) && browser.channel === 'stable'
        },
      })
    })

    it('preselects browser that is provided through the command line', () => {
      cy.openProject('launchpad', ['--e2e', '--browser', 'edge'])

      cy.visitLaunchpad()
      stepThroughConfigPages()

      cy.get('h1').should('contain', 'Choose a Browser')

      cy.findByRole('radio', { name: 'Edge v8', checked: true })
    })
Repository: cypress-io/cypress
File: packages/app/src/settings/SettingsContainer.cy.tsx (TypeScript)

Code before:

    cy.findByText(defaultMessages.settingsPage.editor.title).should('be.visible')
    cy.findByText(defaultMessages.settingsPage.proxy.title).should('be.visible')
    cy.findByText(defaultMessages.settingsPage.testingPreferences.title).should('be.visible')
    cy.percySnapshot()

    cy.findByText('Device Settings').click()

    cy.findByText(defaultMessages.settingsPage.editor.title).should('not.exist')
  })

  it('expands and collapses cloud settings', () => {
    mountSettingsContainer()

    cy.contains('Dashboard Settings').click()

    cy.findByText(defaultMessages.settingsPage.projectId.title).scrollIntoView().should('be.visible')
    cy.percySnapshot()
    cy.findByText('Dashboard Settings').click()

    cy.findByText(defaultMessages.settingsPage.projectId.title).should('not.exist')
  })

  it('renders footer with CTA button', () => {
    mountSettingsContainer()
    cy.contains('p', defaultMessages.settingsPage.footer.text)
    cy.contains('a', defaultMessages.settingsPage.footer.button)
    .should('have.attr', 'href', defaultMessages.settingsPage.footer.buttonLink)
  })
})


Review comment (suggestion):
```suggestion
    cy.contains('p', defaultMessages.settingsPage.footer.text.replace('{testingType}', 'E2E'))
```

Code after:

    cy.findByText(defaultMessages.settingsPage.editor.title).should('be.visible')
    cy.findByText(defaultMessages.settingsPage.proxy.title).should('be.visible')
    cy.findByText(defaultMessages.settingsPage.testingPreferences.title).should('be.visible')
    cy.percySnapshot()

    cy.findByText('Device Settings').click()

    cy.findByText(defaultMessages.settingsPage.editor.title).should('not.exist')
  })

  it('expands and collapses cloud settings', () => {
    mountSettingsContainer()

    cy.contains('Dashboard Settings').click()

    cy.findByText(defaultMessages.settingsPage.projectId.title).scrollIntoView().should('be.visible')
    cy.percySnapshot()
    cy.findByText('Dashboard Settings').click()

    cy.findByText(defaultMessages.settingsPage.projectId.title).should('not.exist')
  })

  it('renders footer with CTA button', () => {
    mountSettingsContainer()
    cy.contains('p', defaultMessages.settingsPage.footer.text.replace('{testingType}', 'E2E'))
    cy.contains('a', defaultMessages.settingsPage.footer.button)
    .should('have.attr', 'href', defaultMessages.settingsPage.footer.buttonLink)
  })
})
Repository: cypress-io/cypress
File: packages/app/cypress/e2e/runner/runner.ui.cy.ts (TypeScript)

Code before:
        filePath: 'runner/scrolls-command-log.runner.cy.js',
        passCount: 0,
        failCount: 1,
      })

      cy.get('.command-number-column:contains(25)').should('be.visible')
    })

    it('file with empty suites only displays no tests found', () => {
      loadSpec({
        filePath: 'runner/empty-suites.runner.cy.js',
        passCount: 0,
        failCount: 0,
      })

      cy.get('.reporter').contains('No tests found')
    })
  })

  describe('reporter interaction', () => {
    // https://github.com/cypress-io/cypress/issues/8621
    it('user can stop test execution', () => {
      loadSpec({
        filePath: 'runner/stop-execution.runner.cy.js',
        passCount: 0,
        failCount: 1,
      })

      cy.get('.runnable-err-message').should('not.contain', 'ran afterEach even though specs were stopped')
      cy.get('.runnable-err-message').should('contain', 'Cypress test was stopped while running this command.')
    })
  })
})


Review comment (bug):
Passing the `passCount` or `failCount` causes `loadSpec` to wait until the spec has finished but we need to stop the spec so they were removed.

Code after:
        filePath: 'runner/scrolls-command-log.runner.cy.js',
        passCount: 0,
        failCount: 1,
      })

      cy.get('.command-number-column:contains(25)').should('be.visible')
    })

    it('file with empty suites only displays no tests found', () => {
      loadSpec({
        filePath: 'runner/empty-suites.runner.cy.js',
        passCount: 0,
        failCount: 0,
      })

      cy.get('.reporter').contains('No tests found')
    })
  })

  describe('reporter interaction', () => {
    // https://github.com/cypress-io/cypress/issues/8621
    it('user can stop test execution', () => {
      loadSpec({
        filePath: 'runner/stop-execution.runner.cy.js',
      })

      // Click the stop button to stop execution
      cy.get('.stop').click()

      // Verify the UI updates immediately - stop button disappears, restart appears
      cy.get('.stop', { timeout: 100 }).should('not.exist')
      cy.get('.restart', { timeout: 100 }).should('be.visible')

      cy.get('.runnable-err-message').should('not.contain', 'ran afterEach even though specs were stopped')
      cy.get('.runnable-err-message').should('contain', 'Cypress test was stopped while running this command.')
    })
  })
})
Repository: cypress-io/cypress
File: packages/app/src/store/studio-store.ts (TypeScript)

Code before:
    },

    setTestId (testId: string) {
      this.testId = testId
      this.suiteId = undefined
      this.newTestLineNumber = undefined
      this._updateUrlParams(['testId', 'suiteId', 'newTestLineNumber'])
    },

    setSuiteId (suiteId: string) {
      this.suiteId = suiteId
      this.testId = undefined
      this._updateUrlParams(['testId', 'suiteId'])
    },

    setCanAccessStudioAI (canAccessStudioAI: boolean) {
      this.canAccessStudioAI = canAccessStudioAI
    },

    setCloudStudioSessionId (cloudStudioSessionId: string) {
      this.cloudStudioSessionId = cloudStudioSessionId
    },

    setNewTestLineNumber (newTestLineNumber: number) {
      this.newTestLineNumber = newTestLineNumber
      this._updateUrlParams(['newTestLineNumber'])
    },

    clearRunnableIds () {
      this.testId = undefined
      this.suiteId = undefined
    },

    openInstructionModal () {
      this.instructionModalIsOpen = true
    },

    closeInstructionModal () {
      this.instructionModalIsOpen = false
    },

    showSaveModal () {
      this.saveModalIsOpen = true
    },

    closeSaveModal () {
      this.saveModalIsOpen = false
    },

    startLoading () {
      this.isLoading = true

Review comment (suggestion):
setNewTestLineNumber does not clear existing `testId` or `suiteId`, which can result in conflicting Studio state; consider resetting those fields when starting a new-test flow.
```suggestion
      this.testId = undefined
      this.suiteId = undefined
      this.newTestLineNumber = newTestLineNumber
      this._updateUrlParams(['testId', 'suiteId', 'newTestLineNumber'])
```

Code after:
    },

    setTestId (testId: string) {
      this.testId = testId
      this.suiteId = undefined
      this.newTestLineNumber = undefined
      this._updateUrlParams(['testId', 'suiteId', 'newTestLineNumber'])
    },

    setSuiteId (suiteId: string) {
      this.suiteId = suiteId
      this.testId = undefined
      this._updateUrlParams(['testId', 'suiteId'])
    },

    setCanAccessStudioAI (canAccessStudioAI: boolean) {
      this.canAccessStudioAI = canAccessStudioAI
    },

    setCloudStudioSessionId (cloudStudioSessionId: string) {
      this.cloudStudioSessionId = cloudStudioSessionId
    },

    setNewTestLineNumber (newTestLineNumber: number) {
      this.newTestLineNumber = newTestLineNumber
      this._updateUrlParams(['newTestLineNumber'])
    },

    clearRunnableIds () {
      this.testId = undefined
      this.suiteId = undefined
      this.newTestLineNumber = undefined
    },

    openInstructionModal () {
      this.instructionModalIsOpen = true
    },

    closeInstructionModal () {
      this.instructionModalIsOpen = false
    },

    showSaveModal () {
      this.saveModalIsOpen = true
    },

    closeSaveModal () {
      this.saveModalIsOpen = false
    },

    startLoading () {
Repository: cypress-io/cypress
File: packages/app/src/store/studio-store.ts (TypeScript)

Code before:
      }

      if (studio.url) {
        this._initialUrl = studio.url
      }

      if (studio.newTestLineNumber) {
        this.setNewTestLineNumber(studio.newTestLineNumber)
      }

      // if we have an existing test or are creating a new test, we need to start loading
      // otherwise if we have a suite, we can just set the studio active
      if (this.testId || studio.newTestLineNumber) {
        this.setAbsoluteFile(config.spec.absolute)
        this.startLoading()
      } else if (this.suiteId) {
        this.setActive(true)
      }
    },

    initialize () {
      if (this.newTestLineNumber) {
        getCypress().runner.setNewTestLineNumber(this.newTestLineNumber)
      } else {
        if (this.suiteId) {
          getCypress().runner.setOnlySuiteId(this.suiteId)
        } else if (this.testId) {
          getCypress().runner.setOnlyTestId(this.testId)
        }
      }
    },

    interceptTest (test) {
      // if this test is the one we created, we can just set the test id
      if (test.invocationDetails?.line === this.newTestLineNumber) {
        this.setTestId(test.id)
      }

      if (this.testId) {
        if (test.invocationDetails) {
          this.setFileDetails(test.invocationDetails)
        }

        this.setRunnableTitle(test.title)
      }
    },

    start (body: HTMLBodyElement) {
      this.isActive = true
      this.isLoading = false
      this.logs = []

Review comment (suggestion):
I think this can be removed. It doesn't look like we're doing anything anymore when we set `setOnlySuiteId`

Code after:
        this.setSuiteId(studio.suiteId)
      }

      if (studio.url) {
        this._initialUrl = studio.url
      }

      // if we have an existing test or are creating a new test, we need to start loading
      // otherwise if we have a suite, we can just set the studio active
      if (this.testId || studio.newTestLineNumber) {
        this.setAbsoluteFile(config.spec.absolute)
        this.startLoading()
      } else if (this.suiteId) {
        this.setActive(true)
      }
    },

    initialize () {
      if (this.newTestLineNumber) {
        getCypress().runner.setNewTestLineNumber(this.newTestLineNumber)
      } else if (this.testId) {
        getCypress().runner.setOnlyTestId(this.testId)
      }
    },

    interceptTest (test) {
      // if this test is the one we created, we can just set the test id
      if (this.newTestLineNumber && test.invocationDetails?.line === this.newTestLineNumber) {
        this.setTestId(test.id)
      }

      if (this.testId) {
        if (test.invocationDetails) {
          this.setFileDetails(test.invocationDetails)
        }

        this.setRunnableTitle(test.title)
      }
    },

    start (body: HTMLBodyElement) {
      this.isActive = true
      this.isLoading = false
      this.logs = []
      this._currentId = 1
      this._hasStarted = true

      const autStore = useAutStore()

      if (this._initialUrl || this.url) {
        this.visitUrl(this._initialUrl)
Repository: cypress-io/cypress
File: system-tests/lib/system-tests.ts (TypeScript)

Code before:
    throw new Error('Specified port cannot be on 5566 because it conflicts with --inspect-brk=5566')
  }
}

const startServer = function (obj) {
  const { onServer, port, https } = obj

  ensurePort(port)

  const app = Express()

  const srv = https ? httpsProxy.httpsServer(app) : new http.Server(app)

  allowDestroy(srv)

  app.use(morgan('dev'))

  if (obj.cors) {
    app.use(require('cors')())
  }

  if (obj.static) {
    app.use(Express.static(path.join(__dirname, '../projects/e2e'), {}) as Express.RequestHandler)
  }

  return new Bluebird((resolve, reject) => {
    return srv.listen(port, () => {
      console.log(`listening on port: ${port}`)
      if (typeof onServer === 'function') {
        onServer(app, srv)
      }

      return resolve(srv)
    })
  })
}

const stopServer = (srv) => srv.destroyAsync()

const copy = function (projectPath: string) {
  const ca = process.env.CIRCLE_ARTIFACTS

  debug('Should copy Circle Artifacts?', Boolean(ca))

  if (ca) {
    const videosFolder = path.join(projectPath, 'cypress/videos')
    const screenshotsFolder = path.join(projectPath, 'cypress/screenshots')

    debug('Copying Circle Artifacts', ca, videosFolder, screenshotsFolder)

    const copy = (src, dest) => {

Review comment (suggestion):
```suggestion
  return new Bluebird((resolve) => {
```

Code after:
    throw new Error('Specified port cannot be on 5566 because it conflicts with --inspect-brk=5566')
  }
}

const startServer = function (obj) {
  const { onServer, port, https } = obj

  ensurePort(port)

  const app = Express()

  const srv = https ? httpsProxy.httpsServer(app) : new http.Server(app)

  allowDestroy(srv)

  app.use(morgan('dev'))

  if (obj.cors) {
    app.use(require('cors')())
  }

  if (obj.static) {
    app.use(Express.static(path.join(__dirname, '../projects/e2e'), {}) as Express.RequestHandler)
  }

  return new Bluebird((resolve) => {
    return srv.listen(port, () => {
      console.log(`listening on port: ${port}`)
      if (typeof onServer === 'function') {
        onServer(app, srv)
      }

      return resolve(srv)
    })
  })
}

const stopServer = (srv) => srv.destroyAsync()

const copy = function (projectPath: string) {
  const ca = process.env.CIRCLE_ARTIFACTS

  debug('Should copy Circle Artifacts?', Boolean(ca))

  if (ca) {
    const videosFolder = path.join(projectPath, 'cypress/videos')
    const screenshotsFolder = path.join(projectPath, 'cypress/screenshots')

    debug('Copying Circle Artifacts', ca, videosFolder, screenshotsFolder)

    const copy = (src, dest) => {
Repository: cypress-io/cypress
File: packages/app/src/runner/dom.ts (TypeScript)

Code before:
      container: containers[0],
      vueContainer: shadowRoot.querySelector('.vue-container'),
    }
  }

  // Create container element

  const container = document.createElement('div')

  container.classList.add(className)

  // NOTE: This is needed to prevent the container from inheriting styles from the body of the AUT
  container.style.all = 'initial'
  container.style.position = 'static'

  body.appendChild(container)

  // Create react-container element

  const shadowRoot = container.attachShadow({ mode: 'open' })

  const vueContainer = document.createElement('div')

  vueContainer.classList.add('vue-container')

  // make the shadow dom an overlay so that clicks on the elements under it are not captured
  Object.assign(vueContainer.style, {
    position: 'absolute',
    top: '0',
    left: '0',
    width: '100%',
    height: '100%',
    zIndex: '1000',
  })

  shadowRoot.appendChild(vueContainer)

  // Prepend style element

  const style = document.createElement('style')

  style.innerHTML = css.toString()

  shadowRoot.prepend(style)

  return {
    container,
    vueContainer,
    shadowRoot,
  }
}

Review comment (suggestion):
```suggestion
  // make the shadow dom an overlay so that all clicks on the elements are captured by it
```

Code after:
      container: containers[0],
      vueContainer: shadowRoot.querySelector('.vue-container'),
    }
  }

  // Create container element

  const container = document.createElement('div')

  container.classList.add(className)

  // NOTE: This is needed to prevent the container from inheriting styles from the body of the AUT
  container.style.all = 'initial'
  container.style.position = 'static'

  body.appendChild(container)

  // Create react-container element

  const shadowRoot = container.attachShadow({ mode: 'open' })

  const vueContainer = document.createElement('div')

  vueContainer.classList.add('vue-container')

  if (studioActive) {
  // make the shadow dom an overlay so that all clicks on the elements are captured by it
    Object.assign(vueContainer.style, {
      position: 'absolute',
      top: '0',
      left: '0',
      width: '100%',
      height: '100%',
      zIndex: '9999',
    })
  }

  shadowRoot.appendChild(vueContainer)

  // Prepend style element

  const style = document.createElement('style')

  style.innerHTML = css.toString()

  shadowRoot.prepend(style)

  return {
    container,
    vueContainer,
    shadowRoot,
Repository: cypress-io/cypress
File: packages/app/src/store/studio-store.ts (TypeScript)

Code before:

      possibleAssertions.push({
        type: 'be.visible',
      })

      const isDisabled = $el.prop('disabled')

      if (isDisabled !== undefined) {
        possibleAssertions.push({
          type: isDisabled ? 'be.disabled' : 'be.enabled',
        })
      }

      const isChecked = $el.prop('checked')

      if (isChecked !== undefined) {
        possibleAssertions.push({
          type: isChecked ? 'be.checked' : 'not.be.checked',
        })
      }

      return possibleAssertions
    },
  },

  getters: {
    isOpen: (state) => {
      return state.isActive || state.isLoading || state._hasStarted
    },

    isEmpty: (state): boolean => {
      return state.logs.length === 0
    },

    hookId: (state) => {
      return `${state.testId}-studio`
    },

    needsUrl: (state) => {
      return state.isActive && !state.url && !state.isFailed && state._hasStarted
    },

    testError: (state) => {
      return {
        id: state.testId,
        state: 'failed',
      }
    },
  },
})


Review comment (suggestion):
This was causing the AUT URL to get removed whenever the panel was open at all, even if it was just the new test screen

Code after:

      if (isChecked !== undefined) {
        possibleAssertions.push({
          type: isChecked ? 'be.checked' : 'not.be.checked',
        })
      }

      return possibleAssertions
    },
  },

  getters: {
    isOpen: (state) => {
      return state.isActive || state.isLoading || state._hasStarted
    },

    isEmpty: (state): boolean => {
      return state.logs.length === 0
    },

    hookId: (state) => {
      return `${state.testId}-studio`
    },

    needsUrl: (state) => {
      return state.isActive && !state.url && !state.isFailed && state._hasStarted
    },

    testError: (state) => {
      return {
        id: state.testId,
        state: 'failed',
      }
    },
  },
})