ronantakizawa/codereview-bench huggingface dataset:
Repository: cypress-io/cypress
File: packages/driver/src/dom/elements.ts (TypeScript)

Code before:
const isOptgroup = (el) => {
  return getTagName(el) === 'optgroup'
}

const isBody = (el): el is HTMLBodyElement => {
  return getTagName(el) === 'body'
}

const isIframe = (el) => {
  return getTagName(el) === 'iframe'
}

const isHTML = (el) => {
  return getTagName(el) === 'html'
}

const isSvg = function (el): el is SVGElement {
  try {
    return 'ownerSVGElement' in el
  } catch (error) {
    return false
  }
}

// active element is the default if its null
// or it's equal to document.body
const activeElementIsDefault = (activeElement, body: HTMLElement) => {
  return !activeElement || (activeElement === body && !isContentEditable(body))
}

const isFocused = (el) => {
  try {
    const doc = $document.getDocumentFromElement(el)

    const { activeElement, body } = doc

    if (activeElementIsDefault(activeElement, body)) {
      return false
    }

    return doc.activeElement === el
  } catch (err) {
    return false
  }
}

const isFocusedOrInFocused = (el: HTMLElement) => {
  debug('isFocusedOrInFocus', el)

  const doc = $document.getDocumentFromElement(el)


Review comment (suggestion):
```suggestion
// or it's equal to document.body that is not contenteditable
```

Code after:
const isOptgroup = (el) => {
  return getTagName(el) === 'optgroup'
}

const isBody = (el): el is HTMLBodyElement => {
  return getTagName(el) === 'body'
}

const isIframe = (el) => {
  return getTagName(el) === 'iframe'
}

const isHTML = (el) => {
  return getTagName(el) === 'html'
}

const isSvg = function (el): el is SVGElement {
  try {
    return 'ownerSVGElement' in el
  } catch (error) {
    return false
  }
}

// active element is the default if its null
// or it's equal to document.body that is not contenteditable
const activeElementIsDefault = (activeElement, body: HTMLElement) => {
  return !activeElement || (activeElement === body && !isContentEditable(body))
}

const isFocused = (el) => {
  try {
    const doc = $document.getDocumentFromElement(el)

    const { activeElement, body } = doc

    if (activeElementIsDefault(activeElement, body)) {
      return false
    }

    return doc.activeElement === el
  } catch (err) {
    return false
  }
}

const isFocusedOrInFocused = (el: HTMLElement) => {
  debug('isFocusedOrInFocus', el)

  const doc = $document.getDocumentFromElement(el)
Repository: cypress-io/cypress
File: packages/driver/src/cypress/runner.js (JavaScript)

Code before:
//   tests: {
//     r2: {id: "r2", title: "foo", suiteId: "r1", state: "passed", err: "", routes: [
//         {}, {}
//       ]
//       agents: [
//       ]
//       commands: [
//         {}, {}, {}
//       ]
//     }}
//
//     r3: {id: "r3", title: "bar", suiteId: "r1", state: "failed", logs: {
//       routes: [
//         {}, {}
//       ]
//       spies: [
//       ]
//       commands: [
//         {}, {}, {}
//       ]
//     }}
//   ]
// }

const fire = function (event, runnable, Cypress) {
  debug('fire:', event)
  if (runnable._fired == null) {
    runnable._fired = {}
  }

  runnable._fired[event] = true

  // dont fire anything again if we are skipped
  if (runnable._ALREADY_RAN) {
    return
  }

  return Cypress.action(event, wrap(runnable), runnable)
}

const fired = (event, runnable) => {
  return !!(runnable._fired && runnable._fired[event])
}

const testBeforeRunAsync = (test, Cypress) => {
  return Promise.try(() => {
    if (!fired('runner:test:before:run:async', test)) {
      return fire('runner:test:before:run:async', test, Cypress)
    }
  })
}

Review comment (bug):
```suggestion
  debug('fire: %o', { event })
```

Code after:
//   tests: {
//     r2: {id: "r2", title: "foo", suiteId: "r1", state: "passed", err: "", routes: [
//         {}, {}
//       ]
//       agents: [
//       ]
//       commands: [
//         {}, {}, {}
//       ]
//     }}
//
//     r3: {id: "r3", title: "bar", suiteId: "r1", state: "failed", logs: {
//       routes: [
//         {}, {}
//       ]
//       spies: [
//       ]
//       commands: [
//         {}, {}, {}
//       ]
//     }}
//   ]
// }

const fire = function (event, runnable, Cypress) {
  debug('fire: %o', { event })
  if (runnable._fired == null) {
    runnable._fired = {}
  }

  runnable._fired[event] = true

  // dont fire anything again if we are skipped
  if (runnable._ALREADY_RAN) {
    return
  }

  return Cypress.action(event, wrap(runnable), runnable)
}

const fired = (event, runnable) => {
  return !!(runnable._fired && runnable._fired[event])
}

const testBeforeRunAsync = (test, Cypress) => {
  return Promise.try(() => {
    if (!fired('runner:test:before:run:async', test)) {
      return fire('runner:test:before:run:async', test, Cypress)
    }
  })
}
Repository: cypress-io/cypress
File: packages/server/test/e2e/3_issue_1987.ts (TypeScript)

Code before:
const e2e = require('../support/helpers/e2e')

describe('e2e issue 1987', () => {
  e2e.setup({
    servers: [{
      port: 3434,
      static: true,
    },
    {
      port: 4545,
      static: true,
    }],
  })

  // https://github.com/cypress-io/cypress/issues/1987
  // before/after hooks should not be rerun on top navigation
  e2e.it('can reload during spec run', {
    spec: 'issue_1987_spec.js',
    snapshot: true,
  })
})


Review comment (suggestion):
you mention after hooks, but these aren't really tested in the test

```suggestion
  // before hooks should not be rerun on top navigation
```

Code after:
const e2e = require('../support/helpers/e2e')
const Fixtures = require('../support/helpers/fixtures')

describe('e2e issue 1987', () => {
  e2e.setup({
    servers: [{
      port: 3434,
      static: true,
    },
    {
      port: 4545,
      static: true,
    }],
  })

  // https://github.com/cypress-io/cypress/issues/1987
  // before/after hooks should not be rerun on top navigation
  e2e.it('can reload during spec run', {
    project: Fixtures.projectPath('hooks-after-rerun'),
    spec: 'beforehook-and-test-navigation.js',
    snapshot: true,
  })

  e2e.it('can run proper amount of hooks', {
    project: Fixtures.projectPath('hooks-after-rerun'),
    spec: 'afterhooks.spec.js',
    snapshot: true,
  })
})
Repository: cypress-io/cypress
File: packages/driver/src/cypress/runner.js (JavaScript)

Code before:
  }

  const onLogsById = (l) => {
    return _logsById[l.id] = l
  }

  const getTest = () => {
    return _test
  }

  const setTest = (t) => {
    return _test = t
  }

  const getTestById = function (id) {
    // perf short circuit
    if (!id) {
      return
    }

    return _testsById[id]
  }

  function hasTestAlreadyRun (test) {
    if (Cypress._RESUMED_AT_TEST) {
      if (+test.id.slice(1) < +Cypress._RESUMED_AT_TEST.slice(1)) {
        return true
      }
    }

    return false
  }

  overrideRunnerHook(Cypress, _runner, getTestById, getTest, setTest, getTests)

  return {
    normalizeAll (tests) {
      // if we have an uncaught error then slice out
      // all of the tests and suites and just generate
      // a single test since we received an uncaught
      // error prior to processing any of mocha's tests
      // which could have occurred in a separate support file
      if (_uncaughtFn) {
        _runner.suite.suites = []
        _runner.suite.tests = []

        // create a runnable to associate for the failure
        mocha.createRootTest('An uncaught error was detected outside of a test', _uncaughtFn)
      }

      return normalizeAll(

Review comment (suggestion):
i would add an additional `getId` function since thats what the `slice(1)` is doing, but its completely masked by this implementation. it looks arbitrary otherwise.

Code after:
  }

  const setTestsById = (tbid) => {
    return _testsById = tbid
  }

  const setTests = (t) => {
    return _tests = t
  }

  const getTests = () => {
    return _tests
  }

  const onRunnable = (r) => {
    return _runnables.push(r)
  }

  const onLogsById = (l) => {
    return _logsById[l.id] = l
  }

  const getTest = () => {
    return _test
  }

  const setTest = (t) => {
    return _test = t
  }

  const getTestById = function (id) {
    // perf short circuit
    if (!id) {
      return
    }

    return _testsById[id]
  }

  function hasTestAlreadyRun (test) {
    if (Cypress._RESUMED_AT_TEST) {
      return getOrderFromId(test.id) < getOrderFromId(Cypress._RESUMED_AT_TEST)
    }

    return false
  }

  overrideRunnerHook(Cypress, _runner, getTestById, getTest, setTest, getTests)

  return {
    normalizeAll (tests) {
Repository: cypress-io/cypress
File: packages/driver/src/cypress/runner.js (JavaScript)

Code before:
  }

  const onLogsById = (l) => {
    return _logsById[l.id] = l
  }

  const getTest = () => {
    return _test
  }

  const setTest = (t) => {
    return _test = t
  }

  const getTestById = function (id) {
    // perf short circuit
    if (!id) {
      return
    }

    return _testsById[id]
  }

  function hasTestAlreadyRun (test) {
    if (Cypress._RESUMED_AT_TEST) {
      if (+test.id.slice(1) < +Cypress._RESUMED_AT_TEST.slice(1)) {
        return true
      }
    }

    return false
  }

  overrideRunnerHook(Cypress, _runner, getTestById, getTest, setTest, getTests)

  return {
    normalizeAll (tests) {
      // if we have an uncaught error then slice out
      // all of the tests and suites and just generate
      // a single test since we received an uncaught
      // error prior to processing any of mocha's tests
      // which could have occurred in a separate support file
      if (_uncaughtFn) {
        _runner.suite.suites = []
        _runner.suite.tests = []

        // create a runnable to associate for the failure
        mocha.createRootTest('An uncaught error was detected outside of a test', _uncaughtFn)
      }

      return normalizeAll(

Review comment (suggestion):
you can also just `return (expression)` here instead of explicitly `return true` after the `if(...)`

Code after:
  }

  const setTestsById = (tbid) => {
    return _testsById = tbid
  }

  const setTests = (t) => {
    return _tests = t
  }

  const getTests = () => {
    return _tests
  }

  const onRunnable = (r) => {
    return _runnables.push(r)
  }

  const onLogsById = (l) => {
    return _logsById[l.id] = l
  }

  const getTest = () => {
    return _test
  }

  const setTest = (t) => {
    return _test = t
  }

  const getTestById = function (id) {
    // perf short circuit
    if (!id) {
      return
    }

    return _testsById[id]
  }

  function hasTestAlreadyRun (test) {
    if (Cypress._RESUMED_AT_TEST) {
      return getOrderFromId(test.id) < getOrderFromId(Cypress._RESUMED_AT_TEST)
    }

    return false
  }

  overrideRunnerHook(Cypress, _runner, getTestById, getTest, setTest, getTests)

  return {
    normalizeAll (tests) {
Repository: cypress-io/cypress
File: packages/server/lib/plugins/child/run_plugins.js (JavaScript)

Code before:
    return false
  })

  process.on('unhandledRejection', (event) => {
    const err = (event && event.reason) || event

    debug('unhandled rejection:', util.serializeError(err))
    ipc.send('error', util.serializeError(err))

    return false
  })

  if (!tsRegistered) {
    try {
      const tsPath = resolve.sync('typescript', {
        basedir: projectRoot,
      })

      const tsOptions = tsNodeOptions.getTsNodeOptions(tsPath)

      debug('typescript path: %s', tsPath)
      debug('registering plugins TS with options %o', tsOptions)

      tsnode.register(tsOptions)
    } catch (e) {
      debug('typescript doesn\'t exist. ts-node setup failed.')
      debug('error message: %s', e.message)
    }

    // ensure typescript is only registered once
    tsRegistered = true
  }

  try {
    debug('require pluginsFile')
    plugins = require(pluginsFile)

    // Handle export default () => {}
    if (plugins && typeof plugins.default === 'function') {
      plugins = plugins.default
    }
  } catch (err) {
    debug('failed to require pluginsFile:\n%s', err.stack)
    ipc.send('load:error', 'PLUGINS_FILE_ERROR', pluginsFile, err.stack)

    return
  }

  if (typeof plugins !== 'function') {
    debug('not a function')
    ipc.send('load:error', 'PLUGINS_DIDNT_EXPORT_FUNCTION', pluginsFile, plugins)

Review comment (bug):
```suggestion
      debug(`typescript doesn't exist. ts-node setup failed.`)
```

How about removing `\` with "`"?

Code after:
    return false
  })

  process.on('unhandledRejection', (event) => {
    const err = (event && event.reason) || event

    debug('unhandled rejection:', util.serializeError(err))
    ipc.send('error', util.serializeError(err))

    return false
  })

  if (!tsRegistered) {
    try {
      const tsPath = resolve.sync('typescript', {
        basedir: projectRoot,
      })

      const tsOptions = tsNodeOptions.getTsNodeOptions(tsPath)

      debug('typescript path: %s', tsPath)
      debug('registering plugins TS with options %o', tsOptions)

      tsnode.register(tsOptions)
    } catch (e) {
      debug(`typescript doesn't exist. ts-node setup failed.`)
      debug('error message: %s', e.message)
    }

    // ensure typescript is only registered once
    tsRegistered = true
  }

  try {
    debug('require pluginsFile')
    plugins = require(pluginsFile)

    // Handle export default () => {}
    if (plugins && typeof plugins.default === 'function') {
      plugins = plugins.default
    }
  } catch (err) {
    debug('failed to require pluginsFile:\n%s', err.stack)
    ipc.send('load:error', 'PLUGINS_FILE_ERROR', pluginsFile, err.stack)

    return
  }

  if (typeof plugins !== 'function') {
    debug('not a function')
    ipc.send('load:error', 'PLUGINS_DIDNT_EXPORT_FUNCTION', pluginsFile, plugins)
Repository: cypress-io/cypress
File: packages/server/lib/gui/auth.js (JavaScript)

Code before:
    }

    if (utm) {
      authUrl.query = {
        utm_source: 'Test Runner',
        utm_medium: 'Login Button',
        utm_campaign: 'TR-Dashboard',
        utm_content: utm,
        ...authUrl.query,
      }
    }

    return authUrl.format()
  })
}

const _getOriginFromUrl = (originalUrl) => {
  const parsedUrl = url.parse(originalUrl)

  return url.format(_.pick(parsedUrl, ['protocol', 'slashes', 'hostname', 'port']))
}

/**
 * @returns a promise that is resolved with a user when auth is complete or rejected when it fails
 */
const start = (onMessage, utm_code) => {
  function sendMessage (type, name, arg1) {
    onMessage({
      type,
      name,
      message: errors.getMsgByType(name, arg1),
      browserOpened: authRedirectReached,
    })
  }

  utm = utm_code
  authRedirectReached = false

  return user.getBaseLoginUrl()
  .then((baseLoginUrl) => {
    return _launchServer(baseLoginUrl, sendMessage)
  })
  .then(() => {
    return _buildLoginRedirectUrl(server)
  })
  .then((loginRedirectUrl) => {
    debug('Trying to open native auth to URL ', loginRedirectUrl)

    return _launchNativeAuth(loginRedirectUrl, sendMessage)
    .then(() => {
      debug('openExternal completed')

Review comment (suggestion):
Keep it camelCase until it absolutely doesn't have to be:

```suggestion
const start = (onMessage, utmCode) => {
```

Code after:
    }

    if (utmCode) {
      authUrl.query = {
        utm_source: 'Test Runner',
        utm_medium: 'Login Button',
        utm_campaign: 'TR-Dashboard',
        utm_content: utmCode,
        ...authUrl.query,
      }
    }

    return authUrl.format()
  })
}

const _getOriginFromUrl = (originalUrl) => {
  const parsedUrl = url.parse(originalUrl)

  return url.format(_.pick(parsedUrl, ['protocol', 'slashes', 'hostname', 'port']))
}

/**
 * @returns a promise that is resolved with a user when auth is complete or rejected when it fails
 */
const start = (onMessage, utmCode) => {
  function sendMessage (type, name, arg1) {
    onMessage({
      type,
      name,
      message: errors.getMsgByType(name, arg1),
      browserOpened: authRedirectReached,
    })
  }

  utm = utmCode
  authRedirectReached = false

  return user.getBaseLoginUrl()
  .then((baseLoginUrl) => {
    return _launchServer(baseLoginUrl, sendMessage)
  })
  .then(() => {
    return _buildLoginRedirectUrl(server)
  })
  .then((loginRedirectUrl) => {
    debug('Trying to open native auth to URL ', loginRedirectUrl)

    return _launchNativeAuth(loginRedirectUrl, sendMessage)
    .then(() => {
      debug('openExternal completed')
Repository: cypress-io/cypress
File: cli/lib/errors.js (JavaScript)

Code before:
const chalk = require('chalk')
const { stripIndent, stripIndents } = require('common-tags')
const { merge } = require('ramda')
const la = require('lazy-ass')
const is = require('check-more-types')

const util = require('./util')
const state = require('./tasks/state')

const docsUrl = 'https://on.cypress.io'
const requiredDependenciesUrl = `${docsUrl}/required-dependencies`
const runDocumentationUrl = `${docsUrl}/command-line#cypress-run`

// TODO it would be nice if all error objects could be enforced via types
// to only have description + solution properties

const hr = '----------'

const genericErrorSolution = stripIndent`
  Search for an existing issue or open a GitHub issue at

    ${chalk.blue(util.issuesUrl)}
`

// common errors Cypress application can encounter
const unknownError = {
  description: 'Unknown Cypress CLI error',
  solution: genericErrorSolution,
}

const invalidRunProjectPath = {
  description: 'Invalid --project path',
  solution: stripIndent`
    Please provide a valid project path.

    Learn more about ${chalk.cyan('cypress run')} at:


Review comment (suggestion):
```suggestion
const runDocumentationUrl = `${docsUrl}/cypress-run`
```

Code after:
const chalk = require('chalk')
const { stripIndent, stripIndents } = require('common-tags')
const { merge } = require('ramda')
const la = require('lazy-ass')
const is = require('check-more-types')

const util = require('./util')
const state = require('./tasks/state')

const docsUrl = 'https://on.cypress.io'
const requiredDependenciesUrl = `${docsUrl}/required-dependencies`
const runDocumentationUrl = `${docsUrl}/cypress-run`

// TODO it would be nice if all error objects could be enforced via types
// to only have description + solution properties

const hr = '----------'

const genericErrorSolution = stripIndent`
  Search for an existing issue or open a GitHub issue at

    ${chalk.blue(util.issuesUrl)}
`

// common errors Cypress application can encounter
const unknownError = {
  description: 'Unknown Cypress CLI error',
  solution: genericErrorSolution,
}

const invalidRunProjectPath = {
  description: 'Invalid --project path',
  solution: stripIndent`
    Please provide a valid project path.

    Learn more about ${chalk.cyan('cypress run')} at:
Repository: cypress-io/cypress
File: packages/server/lib/util/validation.js (JavaScript)

Code before:

  for (let k = 0; k < browsers.length; k += 1) {
    const err = isValidBrowser(browsers[k])

    if (err !== true) {
      return `Found an error while validating the \`browsers\` list. ${err}`
    }
  }

  return true
}

const isValidRetriesConfig = (key, value) => {
  const isNullOrNumber = isOneOf([_.isNumber, _.isNull])

  if (
    isNullOrNumber(value)
    || (_.isEqual(_.keys(value), ['runMode', 'openMode']))
      && isNullOrNumber(value.runMode)
      && isNullOrNumber(value.openMode)
  ) {
    return true
  }

  return errMsg(key, value, 'a number or null or an object with keys "openMode" and "runMode" with values of numbers or nulls')
}

const isValidFirefoxGcInterval = (key, value) => {
  const isIntervalValue = (val) => {
    if (isNumber(val)) {
      return val >= 0
    }

    return val == null
  }

  if (isIntervalValue(value)
      || (_.isEqual(_.keys(value), ['runMode', 'openMode'])
          && isIntervalValue(value.runMode)
          && isIntervalValue(value.openMode))) {
    return true
  }

  return errMsg(key, value, 'a positive number or null or an object with "openMode" and "runMode" as keys and positive numbers or nulls as values')
}

const isOneOf = (...values) => {
  return (key, value) => {
    if (values.some((v) => {
      if (typeof value === 'function') {
        return value(v)

Review comment (suggestion):
Think this would read better, with commas instead of "this or this or this"

```suggestion
  return errMsg(key, value, 'a positive number, null, or an object with keys "openMode" and "runMode" with a positive number or null values')
```

Code after:

  for (let k = 0; k < browsers.length; k += 1) {
    const err = isValidBrowser(browsers[k])

    if (err !== true) {
      return `Found an error while validating the \`browsers\` list. ${err}`
    }
  }

  return true
}

const isValidRetriesConfig = (key, value) => {
  const optionalKeys = ['runMode', 'openMode']
  const isValidRetryValue = (val) => _.isNull(val) || (Number.isInteger(val) && val >= 0)
  const optionalKeysAreValid = (val, k) => optionalKeys.includes(k) && isValidRetryValue(val)

  if (isValidRetryValue(value)) {
    return true
  }

  if (_.isObject(value) && _.every(value, optionalKeysAreValid)) {
    return true
  }

  return errMsg(key, value, 'a positive number or null or an object with keys "openMode" and "runMode" with values of numbers or nulls')
}

const isValidFirefoxGcInterval = (key, value) => {
  const isIntervalValue = (val) => {
    if (isNumber(val)) {
      return val >= 0
    }

    return val == null
  }

  if (isIntervalValue(value)
      || (_.isEqual(_.keys(value), ['runMode', 'openMode'])
          && isIntervalValue(value.runMode)
          && isIntervalValue(value.openMode))) {
    return true
  }

  return errMsg(key, value, 'a positive number or null or an object with "openMode" and "runMode" as keys and positive numbers or nulls as values')
}

const isOneOf = (...values) => {
  return (key, value) => {
    if (values.some((v) => {
      if (typeof value === 'function') {
Repository: cypress-io/cypress
File: packages/desktop-gui/src/specs/specs-store.js (JavaScript)

Code before:
import _ from 'lodash'
import { action, computed, observable } from 'mobx'
import path from 'path'

import localData from '../lib/local-data'
import Spec from './spec-model'
import Folder from './folder-model'

const pathSeparatorRe = /[\\\/]/g

export const allIntegrationSpecsSpec = new Spec({
  name: 'All Specs',
  absolute: '__all',
  relative: '__all',
  displayName: 'Run all specs',
  specType: 'integration',
})

export const allComponentSpecsSpec = new Spec({
  name: 'All Component Specs',
  absolute: '__all',
  relative: '__all',
  displayName: 'Run all component specs',
  specType: 'component',
})

const formRelativePath = (spec) => {
  return spec === spec.type ? path.join(spec.type, spec.name) : spec.relative
}

const pathsEqual = (path1, path2) => {
  if (!path1 || !path2) return false

  return path1.replace(pathSeparatorRe, '') === path2.replace(pathSeparatorRe, '')
}

/**

Review comment (suggestion):
Should probably also say 'All Integration Specs' here to match the logic of component spec label, otherwise it's not totally accurate when you have both.

Code after:
import _ from 'lodash'
import { action, computed, observable } from 'mobx'
import path from 'path'

import localData from '../lib/local-data'
import Spec from './spec-model'
import Folder from './folder-model'

const pathSeparatorRe = /[\\\/]/g

export const allIntegrationSpecsSpec = new Spec({
  name: 'All Integration Specs',
  absolute: '__all',
  relative: '__all',
  displayName: 'Run all specs',
  specType: 'integration',
})

export const allComponentSpecsSpec = new Spec({
  name: 'All Component Specs',
  absolute: '__all',
  relative: '__all',
  displayName: 'Run all component specs',
  specType: 'component',
})

const formRelativePath = (spec) => {
  return spec.relative
}

const pathsEqual = (path1, path2) => {
  if (!path1 || !path2) return false

  return path1.replace(pathSeparatorRe, '') === path2.replace(pathSeparatorRe, '')
}

/**