ronantakizawa/codereview-bench huggingface dataset:
Repository: cypress-io/cypress
File: npm/vite-dev-server/src/makeCypressPlugin.ts (TypeScript)

Code before:
import { resolve, posix, sep } from 'path'
import { readFile } from 'fs'
import { promisify } from 'util'
import Debug from 'debug'
import { ModuleNode, Plugin, ViteDevServer } from 'vite'

const debug = Debug('cypress:vite-dev-server:plugin')

const read = promisify(readFile)

const pluginName = 'cypress-transform-html'
const OSSepRE = new RegExp(`\\${sep}`, 'g')

function convertPathToPosix (path: string): string {
  return sep === '/'
    ? path
    : path.replace(OSSepRE, '/')
}

const INIT_FILEPATH = resolve(__dirname, '../client/initCypressTests.js')

const HMR_DEPENDENCY_LOOKUP_MAX_ITERATION = 50

function getSpecsPathsSet (specs: Spec[], supportFilePath) {
  return new Set<string>([...specs.map((spec) => spec.absolute), supportFilePath])
}

interface Spec{
  absolute: string
  relative: string
}

export const makeCypressPlugin = (
  projectRoot: string,
  supportFilePath: string,
  devServerEvents: EventEmitter,
  specs: Spec[],
): Plugin => {
  let base = '/'

  let specsPathsSet = getSpecsPathsSet(specs, supportFilePath)

  devServerEvents.on('dev-server:specs:changed', (specs: Spec[]) => {
    specsPathsSet = getSpecsPathsSet(specs, supportFilePath)
  })

  const posixSupportFilePath = supportFilePath ? convertPathToPosix(resolve(projectRoot, supportFilePath)) : undefined

  const normalizedSupportFilePath = posixSupportFilePath ? `${base}@fs/${posixSupportFilePath}` : undefined

Review comment (suggestion):
Let's type it - I think the correct type is `supportFile?: string | null`, so you might need a null check, too.

Code after:
import { resolve, posix, sep } from 'path'
import { readFile } from 'fs'
import { promisify } from 'util'
import Debug from 'debug'
import { ModuleNode, Plugin, ViteDevServer } from 'vite'

const debug = Debug('cypress:vite-dev-server:plugin')

const read = promisify(readFile)

const pluginName = 'cypress-transform-html'
const OSSepRE = new RegExp(`\\${sep}`, 'g')

function convertPathToPosix (path: string): string {
  return sep === '/'
    ? path
    : path.replace(OSSepRE, '/')
}

const INIT_FILEPATH = resolve(__dirname, '../client/initCypressTests.js')

const HMR_DEPENDENCY_LOOKUP_MAX_ITERATION = 50

function getSpecsPathsSet (specs: Spec[], supportFile?: string | null) {
  return new Set<string>(
    supportFile
      ? [...specs.map((spec) => spec.absolute), supportFile]
      : specs.map((spec) => spec.absolute),
  )
}

interface Spec{
  absolute: string
  relative: string
}

export const makeCypressPlugin = (
  projectRoot: string,
  supportFilePath: string,
  devServerEvents: EventEmitter,
  specs: Spec[],
): Plugin => {
  let base = '/'

  let specsPathsSet = getSpecsPathsSet(specs, supportFilePath)

  devServerEvents.on('dev-server:specs:changed', (specs: Spec[]) => {
    specsPathsSet = getSpecsPathsSet(specs, supportFilePath)
  })
Repository: cypress-io/cypress
File: packages/launchpad/src/settings/SettingsCard.vue (Vue)

Code before:
<template>
  <div class="p-4 min-w-650px mx-auto my-0">
    <Disclosure as="section" class="border-1 border-gray-300 rounded" v-slot="{ open }">
      <DisclosureButton data-testid="settings-card-header" class="settings-card-header bg-cool-gray-100 py-4 pl-6 pr-3 select-none cursor-pointer grid gap-4">
        <span class="grid self-center h-full">
          <Icon :icon="icon" class="self-center text-indigo-600 text-xl"/>
        </span>
        <div class="w-0.5px bg-gray-300 h-full"/>
        <div>
          <h2 data-testid="settings-card-title" class="col-start-3 text-xl text-indigo-600">{{ title }}</h2>
          <p data-testid="settings-card-description" class="text-gray-500 text-sm">{{ description }}</p>
        </div>
        <Icon :icon="IconCaret" class="transform transition-transform col-start-4 self-center text-xl text-gray-500" :class="{ 'rotate-90': !open, 'rotate-180': open }"/>
      </DisclosureButton>

      <!-- Content of the Settings Card -->
      <DisclosurePanel class="pl-6 pr-10 pt-6 pb-6 border-t-width-1px border-gray-300">
        <slot/>
      </DisclosurePanel>
    </Disclosure>
  </div>
</template>

<script lang="ts" setup>
  import IconCaret from 'virtual:vite-icons/mdi/caret'
  import Icon from '../components/icon/Icon.vue'
  import type { FunctionalComponent, SVGAttributes } from 'vue'
  import { Disclosure, DisclosureButton, DisclosurePanel } from '@headlessui/vue'
  

Review comment (refactor):
I talk about why we go all-in with utility classes in the final review summary.

```suggestion
      <DisclosureButton data-testid="settings-card-header" class="w-full settings-card-header bg-cool-gray-100 py-4 pl-6 pr-3 select-none cursor-pointer grid gap-4">
```

Code after:
<template>
  <div class="p-4 min-w-650px mx-auto my-0">
    <Disclosure 
      as="section" 
      #="{ open }"
      class="border-1 border-gray-300 rounded">
      <DisclosureButton 
        data-testid="settings-card-header" 
        class="settings-card-header w-full bg-cool-gray-100 py-4 pl-6 pr-3 select-none cursor-pointer grid gap-4">
        <span class="grid self-center h-full">
          <Icon :icon="icon" class="self-center text-indigo-600 text-xl"/>
        </span>
        <div class="w-0.5px bg-gray-300 h-full"/>
        <div>
          <h2 
            data-testid="settings-card-title" 
            class="col-start-3 text-xl text-indigo-600">
            {{ title }}
          </h2>
          <p data-testid="settings-card-description" class="text-gray-500 text-sm">
            {{ description }}
          </p>
        </div>
        <Icon 
          :icon="IconCaret" 
          class="transform transition-transform col-start-4 self-center text-xl text-gray-500" 
          :class="{ 'rotate-90': !open, 'rotate-180': open }"/>
      </DisclosureButton>
Repository: cypress-io/cypress
File: packages/launchpad/src/settings/SettingsCard.vue (Vue)

Code before:
<template>
  <div class="p-4 min-w-650px mx-auto my-0">
    <Disclosure as="section" class="border-1 border-gray-300 rounded" v-slot="{ open }">
      <DisclosureButton data-testid="settings-card-header" class="settings-card-header bg-cool-gray-100 py-4 pl-6 pr-3 select-none cursor-pointer grid gap-4">
        <span class="grid self-center h-full">
          <Icon :icon="icon" class="self-center text-indigo-600 text-xl"/>
        </span>
        <div class="w-0.5px bg-gray-300 h-full"/>
        <div>
          <h2 data-testid="settings-card-title" class="col-start-3 text-xl text-indigo-600">{{ title }}</h2>
          <p data-testid="settings-card-description" class="text-gray-500 text-sm">{{ description }}</p>
        </div>
        <Icon :icon="IconCaret" class="transform transition-transform col-start-4 self-center text-xl text-gray-500" :class="{ 'rotate-90': !open, 'rotate-180': open }"/>
      </DisclosureButton>

      <!-- Content of the Settings Card -->
      <DisclosurePanel class="pl-6 pr-10 pt-6 pb-6 border-t-width-1px border-gray-300">
        <slot/>
      </DisclosurePanel>
    </Disclosure>
  </div>
</template>

<script lang="ts" setup>
  import IconCaret from 'virtual:vite-icons/mdi/caret'
  import Icon from '../components/icon/Icon.vue'
  import type { FunctionalComponent, SVGAttributes } from 'vue'
  import { Disclosure, DisclosureButton, DisclosurePanel } from '@headlessui/vue'

Review comment (suggestion):
I have a personal preference to use the hash syntax for scoped slots. I find it's more readable when dealing with named scoped slots and I want to pick one way of writing scoped slots.

That being said, so long as we pick a single way of doing it, I don't care.

```suggestion
    <Disclosure as="section" class="border-1 border-gray-300 rounded" #="{ open }">
```

Code after:
<template>
  <div class="p-4 min-w-650px mx-auto my-0">
    <Disclosure 
      as="section" 
      #="{ open }"
      class="border-1 border-gray-300 rounded">
      <DisclosureButton 
        data-testid="settings-card-header" 
        class="settings-card-header w-full bg-cool-gray-100 py-4 pl-6 pr-3 select-none cursor-pointer grid gap-4">
        <span class="grid self-center h-full">
          <Icon :icon="icon" class="self-center text-indigo-600 text-xl"/>
        </span>
        <div class="w-0.5px bg-gray-300 h-full"/>
        <div>
          <h2 
            data-testid="settings-card-title" 
            class="col-start-3 text-xl text-indigo-600">
            {{ title }}
          </h2>
          <p data-testid="settings-card-description" class="text-gray-500 text-sm">
            {{ description }}
          </p>
        </div>
        <Icon 
          :icon="IconCaret" 
          class="transform transition-transform col-start-4 self-center text-xl text-gray-500" 
          :class="{ 'rotate-90': !open, 'rotate-180': open }"/>
      </DisclosureButton>
Repository: cypress-io/cypress
File: cli/types/cypress.d.ts (TypeScript)

Code before:
    testFiles: string | string[]
    /**
     * The user agent the browser sends in all request headers.
     */
    userAgent: null | string
    /**
     * Polyfills `window.fetch` to enable Network spying and stubbing
     */
    experimentalFetchPolyfill: boolean

    /**
     * Override default config options for Component Testing runner.
     * @default {}
     */
    component: Omit<ResolvedConfigOptions, TestingType>

    /**
     * Override default config options for E2E Testing runner.
     * @default {}
     */
    e2e: Omit<ResolvedConfigOptions, TestingType>

    /**
     * An array of objects defining the certificates
     */
    clientCertificate: ClientCertificate[]
  }

  /**
   * Options appended to config object on runtime.
   */
  interface RuntimeConfigOptions {
    /**
     * CPU architecture, from Node `os.arch()`
     *
     * @see https://nodejs.org/api/os.html#os_os_arch
     */
    arch: string
    /**
     * The browser Cypress is running on.
     */
    browser: Browser
    /**
     * Available browsers found on your system.
     */
    browsers: Browser[]
    /**
     * Hosts mappings to IP addresses.
     */
    hosts: null | string[]
    /**

Review comment (suggestion):
```suggestion
    clientCertificates: ClientCertificate[]
```

Code after:
    testFiles: string | string[]
    /**
     * The user agent the browser sends in all request headers.
     */
    userAgent: null | string
    /**
     * Polyfills `window.fetch` to enable Network spying and stubbing
     */
    experimentalFetchPolyfill: boolean

    /**
     * Override default config options for Component Testing runner.
     * @default {}
     */
    component: Omit<ResolvedConfigOptions, TestingType>

    /**
     * Override default config options for E2E Testing runner.
     * @default {}
     */
    e2e: Omit<ResolvedConfigOptions, TestingType>

    /**
     * An array of objects defining the certificates
     */
    clientCertificates: ClientCertificate[]
  }

  /**
   * Options appended to config object on runtime.
   */
  interface RuntimeConfigOptions {
    /**
     * CPU architecture, from Node `os.arch()`
     *
     * @see https://nodejs.org/api/os.html#os_os_arch
     */
    arch: string
    /**
     * The browser Cypress is running on.
     */
    browser: Browser
    /**
     * Available browsers found on your system.
     */
    browsers: Browser[]
    /**
     * Hosts mappings to IP addresses.
     */
    hosts: null | string[]
    /**
Repository: cypress-io/cypress
File: packages/server/lib/controllers/files.js (JavaScript)

Code before:
const _ = require('lodash')
const R = require('ramda')
const path = require('path')
const Promise = require('bluebird')
const cwd = require('../cwd')
const glob = require('../util/glob')
const specsUtil = require('../util/specs')
const pathHelpers = require('../util/path_helpers')
const debug = require('debug')('cypress:server:controllers')
const { escapeFilenameInUrl } = require('../util/escape_filename')

const SPEC_URL_PREFIX = '/__cypress/tests?p'

module.exports = {
  handleFiles (req, res, config) {
    debug('handle files')

    return specsUtil.find(config)
    .then((files) => {
      return res.json({
        integration: files,
      })
    })
  },

  handleIframe (req, res, config, getRemoteState, extraOptions) {
    const test = req.params[0]
    const iframePath = cwd('lib', 'html', 'iframe.html')
    const specFilter = _.get(extraOptions, 'specFilter')

    debug('handle iframe %o', { test, specFilter })

    return this.getSpecs(test, config, extraOptions)
    .then((specs) => {
      return this.getSupportFile(config)
      .then((js) => {
        const allFilesToSend = js.concat(specs)

        debug('all files to send %o', _.map(allFilesToSend, 'relative'))

        const iframeOptions = {
          title: this.getTitle(test),
          domain: getRemoteState().domainName,

Review comment (suggestion):
Not really ideal to need to call `default` but not having the `default` export means some tests need reworking slightly, and I want to keep the changes to a minimum, so I just did it like this.

Code after:
const _ = require('lodash')
const R = require('ramda')
const path = require('path')
const Promise = require('bluebird')
const cwd = require('../cwd')
const glob = require('../util/glob')
const specsUtil = require('../util/specs')
const pathHelpers = require('../util/path_helpers')
const debug = require('debug')('cypress:server:controllers')
const { escapeFilenameInUrl } = require('../util/escape_filename')

const SPEC_URL_PREFIX = '/__cypress/tests?p'

module.exports = {
  handleFiles (req, res, config) {
    debug('handle files')

    return specsUtil.default.findSpecs(config)
    .then((files) => {
      return res.json({
        integration: files,
      })
    })
  },

  handleIframe (req, res, config, getRemoteState, extraOptions) {
    const test = req.params[0]
    const iframePath = cwd('lib', 'html', 'iframe.html')
    const specFilter = _.get(extraOptions, 'specFilter')

    debug('handle iframe %o', { test, specFilter })

    return this.getSpecs(test, config, extraOptions)
    .then((specs) => {
      return this.getSupportFile(config)
      .then((js) => {
        const allFilesToSend = js.concat(specs)

        debug('all files to send %o', _.map(allFilesToSend, 'relative'))

        const iframeOptions = {
          title: this.getTitle(test),
          domain: getRemoteState().domainName,
Repository: cypress-io/cypress
File: npm/react/plugins/next/findNextWebpackConfig.js (JavaScript)

Code before:
// @ts-check
/// <reference types="next" />
const debug = require('debug')('@cypress/react')
const getNextJsBaseWebpackConfig = require('next/dist/build/webpack-config').default
const { findPagesDir } = require('../../dist/next/findPagesDir')

async function getNextWebpackConfig (config) {
  let loadConfig

  try {
    loadConfig = require('next/dist/next-server/server/config').default
  } catch (e) {
    if (e.code === 'MODULE_NOT_FOUND') {
      // Starting from 11.0.2-canary.23, the server config file
      // is not in the next-server folder anymore.
      // @ts-ignore
      loadConfig = require('next/dist/server/config').default
    } else {
      throw e
    }
  }
  const nextConfig = await loadConfig('development', config.projectRoot)
  const nextWebpackConfig = await getNextJsBaseWebpackConfig(
    config.projectRoot,
    {
      buildId: `@cypress/react-${Math.random().toString()}`,
      config: nextConfig,
      dev: true,
      isServer: false,
      pagesDir: findPagesDir(config.projectRoot),
      entrypoints: {},

Review comment (refactor):
Can we avoid referencing the `dist` folder from the plugins?
My 2 reasons:
- `dist` does not always exist (sometimes it's called `out` sometimes build)
- It took me a good 10 min to fin out why I could not test your repo as I thought plugins were not transpiled.

If we choose to transpile the plugins. We should do it entirely.
I believe this is confusing.

Instead could you replace your ts code with js code.

Another piece of info you might need: @cowboy and the DX team plan on moving all those "plugins" in their own package as soon as they can. I would hold off refactoring everything too fast.

Code after:
// @ts-check
/// <reference types="next" />
const debug = require('debug')('@cypress/react')
const getNextJsBaseWebpackConfig = require('next/dist/build/webpack-config').default
const { findPagesDir } = require('../../dist/next/findPagesDir')
const { getRunWebpackSpan } = require('../../dist/next/getRunWebpackSpan')

async function getNextWebpackConfig (config) {
  let loadConfig

  try {
    loadConfig = require('next/dist/next-server/server/config').default
  } catch (e) {
    if (e.code === 'MODULE_NOT_FOUND') {
      // Starting from 11.0.2-canary.23, the server config file
      // is not in the next-server folder anymore.
      // @ts-ignore
      loadConfig = require('next/dist/server/config').default
    } else {
      throw e
    }
  }
  const nextConfig = await loadConfig('development', config.projectRoot)
  const runWebpackSpan = await getRunWebpackSpan()
  const nextWebpackConfig = await getNextJsBaseWebpackConfig(
    config.projectRoot,
    {
      buildId: `@cypress/react-${Math.random().toString()}`,
      config: nextConfig,
      dev: true,
      isServer: false,
Repository: cypress-io/cypress
File: npm/webpack-dev-server/src/startServer.ts (TypeScript)

Code before:
import Debug from 'debug'
import webpack from 'webpack'
import WebpackDevServer from 'webpack-dev-server'
import { makeWebpackConfig, UserWebpackDevServerOptions } from './makeWebpackConfig'
import { webpackDevServerFacts } from './webpackDevServerFacts'

export interface StartDevServer extends UserWebpackDevServerOptions {
  /* this is the Cypress dev server configuration object */
  options: Cypress.DevServerConfig
  /* support passing a path to the user's webpack config */
  webpackConfig?: WebpackConfigurationWithDevServer
  /* base html template to render in AUT */
  template?: string
}

export interface WebpackConfigurationWithDevServer extends webpack.Configuration {
  devServer?: WebpackDevServer.Configuration
}

const debug = Debug('cypress:webpack-dev-server:start')

export async function start ({ webpackConfig: userWebpackConfig, template, options, ...userOptions }: StartDevServer, exitProcess = process.exit): Promise<WebpackDevServer> {
  if (!userWebpackConfig) {
    debug('User did not pass in any webpack configuration')
  }

  const { projectRoot, devServerPublicPathRoute, isTextTerminal } = options.config

  const webpackConfig = await makeWebpackConfig(userWebpackConfig || {}, {
    files: options.specs,
    template,
    projectRoot,
    devServerPublicPathRoute,
    devServerEvents: options.devServerEvents,
    supportFile: options.config.supportFile as string,
    isOpenMode: !isTextTerminal,

Review comment (suggestion):
Can you use `WebpackOptions` here? https://github.com/webpack/webpack/blob/main/declarations/WebpackOptions.d.ts#L737-L761

Code after:
import Debug from 'debug'
import webpack from 'webpack'
import WebpackDevServer from 'webpack-dev-server'
import { makeWebpackConfig, UserWebpackDevServerOptions } from './makeWebpackConfig'
import { webpackDevServerFacts } from './webpackDevServerFacts'

export interface StartDevServer extends UserWebpackDevServerOptions {
  /* this is the Cypress dev server configuration object */
  options: Cypress.DevServerConfig
  /* Base webpack config object used for loading component testing */
  webpackConfig?: WebpackConfigurationWithDevServer
  /* base html template to render in AUT */
  template?: string
}

export interface WebpackConfigurationWithDevServer extends webpack.Configuration {
  devServer?: WebpackDevServer.Configuration
}

const debug = Debug('cypress:webpack-dev-server:start')

export async function start ({ webpackConfig: userWebpackConfig, template, options, ...userOptions }: StartDevServer, exitProcess = process.exit): Promise<WebpackDevServer> {
  if (!userWebpackConfig) {
    debug('User did not pass in any webpack configuration')
  }

  const { projectRoot, devServerPublicPathRoute, isTextTerminal } = options.config

  const webpackConfig = await makeWebpackConfig(userWebpackConfig || {}, {
    files: options.specs,
    template,
    projectRoot,
    devServerPublicPathRoute,
    devServerEvents: options.devServerEvents,
    supportFile: options.config.supportFile as string,
    isOpenMode: !isTextTerminal,
Repository: cypress-io/cypress
File: packages/driver/cypress/integration/commands/assertions_spec.js (JavaScript)

Code before:
const { $, _ } = Cypress

describe('src/cy/commands/assertions', { retries: 2 }, () => {
  before(() => {
    cy
    .visit('/fixtures/jquery.html')
    .then(function (win) {
      this.body = win.document.body.outerHTML
    })
  })

  beforeEach(function () {
    const doc = cy.state('document')

    $(doc.body).empty().html(this.body)
  })

  context('#should', () => {
    beforeEach(function () {
      this.logs = []

      cy.on('log:added', (attrs, log) => {
        this.logs.push(log)
        this.lastLog = log
      })

      return null
    })

Review comment (bug):
```suggestion
describe('src/cy/commands/assertions', () => {
```

I don't think this is working. You can see based on the latest CI run, it's failing but not showing any attempts.

![Screen Shot 2021-10-25 at 9 14 15 AM](https://user-images.githubusercontent.com/1157043/138702518-a816b8cc-5fb9-4454-96f4-4eed6f82473c.png)

If it was retrying, it would look something like:

![Screen Shot 2021-10-25 at 9 15 44 AM](https://user-images.githubusercontent.com/1157043/138702573-cd7b1da2-c076-4ccd-8376-b3251c3df69d.png)

Also, it shouldn't be necessary to enable retries here (or on the other specs in this PR) because they're [supposed to be set globally](https://github.com/cypress-io/cypress/blob/dcefd77b052b2deec0b9ad0bd29cb16635f50d94/packages/driver/cypress/support/defaults.js#L10). We need to figure out why retries aren't taking affect for the driver and put a fix into `develop` for it. I would remove the per-spec retries config from this PR and we can fix it in another PR.


Code after:
const { $, _ } = Cypress

describe('src/cy/commands/assertions', () => {
  before(() => {
    cy
    .visit('/fixtures/jquery.html')
    .then(function (win) {
      this.body = win.document.body.outerHTML
    })
  })

  beforeEach(function () {
    const doc = cy.state('document')

    $(doc.body).empty().html(this.body)
  })

  context('#should', () => {
    beforeEach(function () {
      this.logs = []

      cy.on('log:added', (attrs, log) => {
        this.logs.push(log)
        this.lastLog = log
      })

      return null
    })