ronantakizawa/codereview-bench huggingface dataset:
Repository: puma/puma
File: test/helpers/integration.rb (Ruby)

Code before:
    begin
      if @server.wait_readable(LOG_WAIT_READ) and line = @server&.gets
        @server_log << line
        puts "    #{line}" if log
      end
    rescue StandardError => e
      error_retries += 1
      raise(Minitest::Assertion,  "Waiting for server to log #{match_obj.inspect} raised #{e.class}") if error_retries == LOG_ERROR_QTY
      sleep LOG_ERROR_SLEEP
      retry
    end
    if Process.clock_gettime(Process::CLOCK_MONOTONIC) > time_timeout
      raise Minitest::Assertion, "Timeout waiting for server to log #{match_obj.inspect}"
    end
    line
  end

  def open_client_socket(unix: false, timeout: 3)
    deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout
    retries = 0
    begin
      unix ? UNIXSocket.new(@bind_path) : TCPSocket.new(HOST, @tcp_port)
    rescue Errno::EADDRNOTAVAIL => e
      raise e if Process.clock_gettime(Process::CLOCK_MONOTONIC) >= deadline
      retries += 1
      sleep 0.01 * [retries, 10].min
      retry
    end
  end

  def connect(path = nil, unix: false)
    s = open_client_socket(unix: unix)
    @ios_to_close << s
    s << "GET /#{path} HTTP/1.1\r\n\r\n"
    s
  end

  # use only if all socket writes are fast
  # does not wait for a read
  def fast_connect(path = nil, unix: false)
    s = open_client_socket(unix: unix)
    @ios_to_close << s
    fast_write s, "GET /#{path} HTTP/1.1\r\n\r\n"
    s
  end

  def fast_write(io, str)
    n = 0
    while true
      begin
        n = io.syswrite str

Review comment (suggestion):
```suggestion
      sleep 0.01 * retries.clamp(0, 10)
```

Code after:
    begin
      if @server.wait_readable(LOG_WAIT_READ) and line = @server&.gets
        @server_log << line
        puts "    #{line}" if log
      end
    rescue StandardError => e
      error_retries += 1
      raise(Minitest::Assertion,  "Waiting for server to log #{match_obj.inspect} raised #{e.class}") if error_retries == LOG_ERROR_QTY
      sleep LOG_ERROR_SLEEP
      retry
    end
    if Process.clock_gettime(Process::CLOCK_MONOTONIC) > time_timeout
      raise Minitest::Assertion, "Timeout waiting for server to log #{match_obj.inspect}"
    end
    line
  end

  def open_client_socket(unix: false, timeout: 3)
    deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout
    retries = 0
    begin
      unix ? UNIXSocket.new(@bind_path) : TCPSocket.new(HOST, @tcp_port)
    rescue Errno::EADDRNOTAVAIL => e
      raise e if Process.clock_gettime(Process::CLOCK_MONOTONIC) >= deadline
      retries += 1
      sleep 0.01 * retries.clamp(0, 10)
      retry
    end
  end

  def connect(path = nil, unix: false)
    s = open_client_socket(unix: unix)
    @ios_to_close << s
    s << "GET /#{path} HTTP/1.1\r\n\r\n"
    s
  end

  # use only if all socket writes are fast
  # does not wait for a read
  def fast_connect(path = nil, unix: false)
    s = open_client_socket(unix: unix)
    @ios_to_close << s
    fast_write s, "GET /#{path} HTTP/1.1\r\n\r\n"
    s
  end

  def fast_write(io, str)
    n = 0
    while true
      begin
        n = io.syswrite str
Repository: puppeteer/puppeteer
File: src/common/QueryHandler.ts (TypeScript)

Code before:
export function unregisterCustomQueryHandler(name: string): void {
  if (_queryHandlers.has(name) && !_builtInHandlers.has(name)) {
    _queryHandlers.delete(name);
  }
}

/**
 * @internal
 */
export function customQueryHandlerNames(): string[] {
  return [..._queryHandlers.keys()].filter(
    (name) => !_builtInHandlers.has(name)
  );
}

/**
 * @internal
 */
export function clearCustomQueryHandlers(): void {
  customQueryHandlerNames().forEach(unregisterCustomQueryHandler);
}

/**
 * @internal
 */
export function getQueryHandlerAndSelector(
  selector: string
): { updatedSelector: string; queryHandler: InternalQueryHandler } {
  const hasCustomQueryHandler = /^[a-zA-Z]+\//.test(selector);
  if (!hasCustomQueryHandler)
    return { updatedSelector: selector, queryHandler: _defaultHandler };

  const index = selector.indexOf('/');
  const name = selector.slice(0, index);
  const updatedSelector = selector.slice(index + 1);
  const queryHandler = _queryHandlers.get(name);
  if (!queryHandler)
    throw new Error(
      `Query set to use "${name}", but no query handler of that name was found`
    );

  return {
    updatedSelector,
    queryHandler,
  };
}


Review comment (refactor):
Now that there are multiple entries, let's split this across multiple lines for readability:

```suggestion
const _builtInHandlers = new Map([
  ['aria', ariaHandler],
  ['pierce', pierceHandler],
]);
```

(unless Prettier likes to put it all on one line for some reason?)

Code after:
  queryAll: (element, selector) => {
    const result: Element[] = [];
    const collect = (root: Element | ShadowRoot) => {
      const iter = document.createTreeWalker(root, NodeFilter.SHOW_ELEMENT);
      do {
        const currentNode = iter.currentNode as HTMLElement;
        if (currentNode.shadowRoot) {
          collect(currentNode.shadowRoot);
        }
        if (currentNode instanceof ShadowRoot) {
          continue;
        }
        if (currentNode.matches(selector)) {
          result.push(currentNode);
        }
      } while (iter.nextNode());
    };
    if (element instanceof Document) {
      element = element.documentElement;
    }
    collect(element);
    return result;
  },
});

const _builtInHandlers = new Map([
  ['aria', ariaHandler],
  ['pierce', pierceHandler],
]);
const _queryHandlers = new Map(_builtInHandlers);

/**
 * @internal
 */
export function registerCustomQueryHandler(
  name: string,
  handler: CustomQueryHandler
): void {
  if (_queryHandlers.get(name))
    throw new Error(`A custom query handler named "${name}" already exists`);

  const isValidName = /^[a-zA-Z]+$/.test(name);
  if (!isValidName)
    throw new Error(`Custom query handler names may only contain [a-zA-Z]`);

  const internalHandler = makeQueryHandler(handler);

  _queryHandlers.set(name, internalHandler);
}

/**
Repository: puppeteer/puppeteer
File: test/src/headful.spec.ts (TypeScript)

Code before:
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

import {mkdtemp} from 'fs/promises';
import os from 'os';
import path from 'path';

import expect from 'expect';
import {PuppeteerLaunchOptions} from 'puppeteer-core/internal/node/PuppeteerNode.js';
import {rmSync} from 'puppeteer-core/internal/node/util/fs.js';

import {getTestState, launch} from './mocha-utils.js';

const TMP_FOLDER = path.join(os.tmpdir(), 'pptr_tmp_folder-');

const extensionPath = path.join(__dirname, '../assets', 'simple-extension');
const serviceWorkerExtensionPath = path.join(
  __dirname,
  '..',
  'assets',
  'serviceworkers',
  'extension'
);

describe.skip('headful tests', function () {
  /* These tests fire up an actual browser so let's
   * allow a higher timeout
   */
  this.timeout(20_000);

  let headfulOptions: PuppeteerLaunchOptions | undefined;
  let headlessOptions: PuppeteerLaunchOptions & {headless: boolean};
  let extensionOptions: PuppeteerLaunchOptions & {
    headless: boolean;
    args: string[];
  };
  let forcedOopifOptions: PuppeteerLaunchOptions & {
    headless: boolean;
    devtools: boolean;
    args: string[];
  };
  let devtoolsOptions: PuppeteerLaunchOptions & {
    headless: boolean;
    devtools: boolean;
  };
  const browsers: Array<() => Promise<void>> = [];

  beforeEach(async () => {
    const {server, defaultBrowserOptions} = await getTestState({
      skipLaunch: true,

Review comment (suggestion):
```suggestion
describe('headful tests', function () {
```

Code after:
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

import {mkdtemp} from 'fs/promises';
import os from 'os';
import path from 'path';

import expect from 'expect';
import {PuppeteerLaunchOptions} from 'puppeteer-core/internal/node/PuppeteerNode.js';
import {rmSync} from 'puppeteer-core/internal/node/util/fs.js';

import {getTestState, launch} from './mocha-utils.js';

const TMP_FOLDER = path.join(os.tmpdir(), 'pptr_tmp_folder-');

const extensionPath = path.join(__dirname, '../assets', 'simple-extension');
const serviceWorkerExtensionPath = path.join(
  __dirname,
  '..',
  'assets',
  'serviceworkers',
  'extension'
);

describe('headful tests', function () {
  /* These tests fire up an actual browser so let's
   * allow a higher timeout
   */
  this.timeout(20_000);

  let headfulOptions: PuppeteerLaunchOptions | undefined;
  let headlessOptions: PuppeteerLaunchOptions & {headless: boolean};
  let extensionOptions: PuppeteerLaunchOptions & {
    headless: boolean;
    args: string[];
  };
  let forcedOopifOptions: PuppeteerLaunchOptions & {
    headless: boolean;
    devtools: boolean;
    args: string[];
  };
  let devtoolsOptions: PuppeteerLaunchOptions & {
    headless: boolean;
    devtools: boolean;
  };
  const browsers: Array<() => Promise<void>> = [];

  beforeEach(async () => {
    const {server, defaultBrowserOptions} = await getTestState({
      skipLaunch: true,
Repository: puppeteer/puppeteer
File: test/src/mocha-utils.ts (TypeScript)

Code before:
};

export const createTimeout = <T>(
  n: number,
  value?: T
): Promise<T | undefined> => {
  return new Promise(resolve => {
    setTimeout(() => {
      return resolve(value);
    }, n);
  });
};

const browserCleanupsAfterAll: Array<() => Promise<void>> = [];
const browserCleanups: Array<() => Promise<void>> = [];

const closeLaunched = (storage: Array<() => Promise<void>>) => {
  return async () => {
    let cleanup = storage.pop();
    try {
      while (cleanup) {
        await cleanup();
        cleanup = storage.pop();
      }
    } catch (error) {
      // If the browser was closed by other mean swallow the error
      // and mark he browser as closed
      if ((error as any)?.message.includes('Connection closed')) {
        storage.splice(0, storage.length);
        return;
      }

      throw error;
    }
  };
};

export const launch = async (
  launchOptions: PuppeteerLaunchOptions,
  options: {
    after?: 'each' | 'all';
    createContext?: boolean;
    createPage?: boolean;
  } = {}
): Promise<
  PuppeteerTestState & {
    close: () => Promise<void>;
  }
> => {
  const {after = 'each', createContext = true, createPage = true} = options;
  const initState = await getTestState({

Review comment (suggestion):
```suggestion
      // If the browser was closed by other means, swallow the error
```

Code after:
      return resolve(value);
    }, n);
  });
};

const browserCleanupsAfterAll: Array<() => Promise<void>> = [];
const browserCleanups: Array<() => Promise<void>> = [];

const closeLaunched = (storage: Array<() => Promise<void>>) => {
  return async () => {
    let cleanup = storage.pop();
    try {
      while (cleanup) {
        await cleanup();
        cleanup = storage.pop();
      }
    } catch (error) {
      // If the browser was closed by other means, swallow the error
      // and mark the browser as closed.
      if ((error as Error)?.message.includes('Connection closed')) {
        storage.splice(0, storage.length);
        return;
      }

      throw error;
    }
  };
};

export const launch = async (
  launchOptions: PuppeteerLaunchOptions,
  options: {
    after?: 'each' | 'all';
    createContext?: boolean;
    createPage?: boolean;
  } = {}
): Promise<
  PuppeteerTestState & {
    close: () => Promise<void>;
  }
> => {
  const {after = 'each', createContext = true, createPage = true} = options;
  const initState = await getTestState({
    skipLaunch: true,
  });
  const cleanupStorage =
    after === 'each' ? browserCleanups : browserCleanupsAfterAll;
  try {
    const browser = await puppeteer.launch({
      ...initState.defaultBrowserOptions,
      ...launchOptions,
Repository: puppeteer/puppeteer
File: test/src/mocha-utils.ts (TypeScript)

Code before:

export const createTimeout = <T>(
  n: number,
  value?: T
): Promise<T | undefined> => {
  return new Promise(resolve => {
    setTimeout(() => {
      return resolve(value);
    }, n);
  });
};

const browserCleanupsAfterAll: Array<() => Promise<void>> = [];
const browserCleanups: Array<() => Promise<void>> = [];

const closeLaunched = (storage: Array<() => Promise<void>>) => {
  return async () => {
    let cleanup = storage.pop();
    try {
      while (cleanup) {
        await cleanup();
        cleanup = storage.pop();
      }
    } catch (error) {
      // If the browser was closed by other mean swallow the error
      // and mark he browser as closed
      if ((error as any)?.message.includes('Connection closed')) {
        storage.splice(0, storage.length);
        return;
      }

      throw error;
    }
  };
};

export const launch = async (
  launchOptions: PuppeteerLaunchOptions,
  options: {
    after?: 'each' | 'all';
    createContext?: boolean;
    createPage?: boolean;
  } = {}
): Promise<
  PuppeteerTestState & {
    close: () => Promise<void>;
  }
> => {
  const {after = 'each', createContext = true, createPage = true} = options;
  const initState = await getTestState({
    skipLaunch: true,

Review comment (suggestion):
```suggestion
      // and mark the browser as closed.
```

Code after:
    }, n);
  });
};

const browserCleanupsAfterAll: Array<() => Promise<void>> = [];
const browserCleanups: Array<() => Promise<void>> = [];

const closeLaunched = (storage: Array<() => Promise<void>>) => {
  return async () => {
    let cleanup = storage.pop();
    try {
      while (cleanup) {
        await cleanup();
        cleanup = storage.pop();
      }
    } catch (error) {
      // If the browser was closed by other means, swallow the error
      // and mark the browser as closed.
      if ((error as Error)?.message.includes('Connection closed')) {
        storage.splice(0, storage.length);
        return;
      }

      throw error;
    }
  };
};

export const launch = async (
  launchOptions: PuppeteerLaunchOptions,
  options: {
    after?: 'each' | 'all';
    createContext?: boolean;
    createPage?: boolean;
  } = {}
): Promise<
  PuppeteerTestState & {
    close: () => Promise<void>;
  }
> => {
  const {after = 'each', createContext = true, createPage = true} = options;
  const initState = await getTestState({
    skipLaunch: true,
  });
  const cleanupStorage =
    after === 'each' ? browserCleanups : browserCleanupsAfterAll;
  try {
    const browser = await puppeteer.launch({
      ...initState.defaultBrowserOptions,
      ...launchOptions,
    });
Repository: puppeteer/puppeteer
File: test/src/mocha-utils.ts (TypeScript)

Code before:
export const createTimeout = <T>(
  n: number,
  value?: T
): Promise<T | undefined> => {
  return new Promise(resolve => {
    setTimeout(() => {
      return resolve(value);
    }, n);
  });
};

const browserCleanupsAfterAll: Array<() => Promise<void>> = [];
const browserCleanups: Array<() => Promise<void>> = [];

const closeLaunched = (storage: Array<() => Promise<void>>) => {
  return async () => {
    let cleanup = storage.pop();
    try {
      while (cleanup) {
        await cleanup();
        cleanup = storage.pop();
      }
    } catch (error) {
      // If the browser was closed by other mean swallow the error
      // and mark he browser as closed
      if ((error as any)?.message.includes('Connection closed')) {
        storage.splice(0, storage.length);
        return;
      }

      throw error;
    }
  };
};

export const launch = async (
  launchOptions: PuppeteerLaunchOptions,
  options: {
    after?: 'each' | 'all';
    createContext?: boolean;
    createPage?: boolean;
  } = {}
): Promise<
  PuppeteerTestState & {
    close: () => Promise<void>;
  }
> => {
  const {after = 'each', createContext = true, createPage = true} = options;
  const initState = await getTestState({
    skipLaunch: true,
  });

Review comment (suggestion):
```suggestion
      if ((error as Error)?.message.includes('Connection closed')) {
```

Code after:
  });
};

const browserCleanupsAfterAll: Array<() => Promise<void>> = [];
const browserCleanups: Array<() => Promise<void>> = [];

const closeLaunched = (storage: Array<() => Promise<void>>) => {
  return async () => {
    let cleanup = storage.pop();
    try {
      while (cleanup) {
        await cleanup();
        cleanup = storage.pop();
      }
    } catch (error) {
      // If the browser was closed by other means, swallow the error
      // and mark the browser as closed.
      if ((error as Error)?.message.includes('Connection closed')) {
        storage.splice(0, storage.length);
        return;
      }

      throw error;
    }
  };
};

export const launch = async (
  launchOptions: PuppeteerLaunchOptions,
  options: {
    after?: 'each' | 'all';
    createContext?: boolean;
    createPage?: boolean;
  } = {}
): Promise<
  PuppeteerTestState & {
    close: () => Promise<void>;
  }
> => {
  const {after = 'each', createContext = true, createPage = true} = options;
  const initState = await getTestState({
    skipLaunch: true,
  });
  const cleanupStorage =
    after === 'each' ? browserCleanups : browserCleanupsAfterAll;
  try {
    const browser = await puppeteer.launch({
      ...initState.defaultBrowserOptions,
      ...launchOptions,
    });
    cleanupStorage.push(() => {
Repository: puppeteer/puppeteer
File: packages/puppeteer-core/src/common/bidi/BrowsingContext.ts (TypeScript)

Code before:
  }

  connection(): CDPConnection | undefined {
    return undefined;
  }

  async send<T extends keyof ProtocolMapping.Commands>(
    method: T,
    ...paramArgs: ProtocolMapping.Commands[T]['paramsType']
  ): Promise<ProtocolMapping.Commands[T]['returnType']> {
    if (this.#detached) {
      throw new TargetCloseError(
        `Protocol error (${method}): Session closed. Most likely the page has been closed.`
      );
    }
    const session = await this.#sessionId.valueOrThrow();
    const result = await this.#context.connection.send('cdp.sendCommand', {
      method: method,
      params: paramArgs[0],
      session,
    });
    return result.result;
  }

  async detach(): Promise<void> {
    cdpSessions.set(this.id(), this);
    await this.#context.cdpSession.send('Target.detachFromTarget', {
      sessionId: this.id(),
    });
    this.#detached = true;
  }

  id(): string {
    const val = this.#sessionId.value();
    return val instanceof Error || val === undefined ? '' : val;
  }
}

/**
 * @internal
 */
export class BrowsingContext extends Realm {
  #timeoutSettings: TimeoutSettings;
  #id: string;
  #url: string;
  #cdpSession: CDPSession;

  constructor(
    connection: Connection,
    timeoutSettings: TimeoutSettings,
    info: Bidi.BrowsingContext.Info

Review comment (suggestion):
```suggestion
    cdpSessions.delete(this.id());
```

Code after:
  }

  connection(): CDPConnection | undefined {
    return undefined;
  }

  async send<T extends keyof ProtocolMapping.Commands>(
    method: T,
    ...paramArgs: ProtocolMapping.Commands[T]['paramsType']
  ): Promise<ProtocolMapping.Commands[T]['returnType']> {
    if (this.#detached) {
      throw new TargetCloseError(
        `Protocol error (${method}): Session closed. Most likely the page has been closed.`
      );
    }
    const session = await this.#sessionId.valueOrThrow();
    const result = await this.#context.connection.send('cdp.sendCommand', {
      method: method,
      params: paramArgs[0],
      session,
    });
    return result.result;
  }

  async detach(): Promise<void> {
    cdpSessions.delete(this.id());
    await this.#context.cdpSession.send('Target.detachFromTarget', {
      sessionId: this.id(),
    });
    this.#detached = true;
  }

  id(): string {
    const val = this.#sessionId.value();
    return val instanceof Error || val === undefined ? '' : val;
  }
}

/**
 * @internal
 */
export class BrowsingContext extends Realm {
  #timeoutSettings: TimeoutSettings;
  #id: string;
  #url: string;
  #cdpSession: CDPSession;

  constructor(
    connection: Connection,
    timeoutSettings: TimeoutSettings,
    info: Bidi.BrowsingContext.Info
Repository: puppeteer/puppeteer
File: tools/mochaRunner/src/test.ts (TypeScript)

Code before:
        testIdMatchesExpectationPattern(test, pattern),
        expected,
        `Expected "${pattern}" to yield "${expected}"`
      );
    }
  });
});

describe('getExpectationUpdates', () => {
  it('should generate an update for expectations if a test passed with a fail expectation', () => {
    const mochaResults = {
      stats: {tests: 1},
      pending: [],
      passes: [
        {
          fullTitle: 'Page Page.setContent should work',
          title: 'should work',
          file: 'page.spec.ts',
        },
      ],
      failures: [],
    };
    const expectations = [
      {
        testIdPattern: '[page.spec] Page Page.setContent should work',
        platforms: [os.platform()],
        parameters: ['test'],
        expectations: ['FAIL' as const],
      },
    ];
    const updates = getExpectationUpdates(mochaResults, expectations, {
      platforms: [os.platform()],
      parameters: ['test'],
    });
    assert.deepEqual(updates, [
      {
        action: 'remove',
        basedOn: {
          expectations: ['FAIL'],
          parameters: ['test'],
          platforms: ['darwin'],
          testIdPattern: '[page.spec] Page Page.setContent should work',
        },
        expectation: {
          expectations: ['FAIL'],
          parameters: ['test'],
          platforms: ['darwin'],
          testIdPattern: '[page.spec] Page Page.setContent should work',
        },
      },
    ]);

Review comment (suggestion):
```suggestion
        platforms: ['darwin'] as Platform[],
```

Code after:
        expected,
        `Expected "${pattern}" to yield "${expected}"`
      );
    }
  });
});

describe('getExpectationUpdates', () => {
  it('should generate an update for expectations if a test passed with a fail expectation', () => {
    const mochaResults = {
      stats: {tests: 1},
      pending: [],
      passes: [
        {
          fullTitle: 'Page Page.setContent should work',
          title: 'should work',
          file: 'page.spec.ts',
        },
      ],
      failures: [],
    };
    const expectations = [
      {
        testIdPattern: '[page.spec] Page Page.setContent should work',
        platforms: ['darwin'] as Platform[],
        parameters: ['test'],
        expectations: ['FAIL' as const],
      },
    ];
    const updates = getExpectationUpdates(mochaResults, expectations, {
      platforms: ['darwin'] as Platform[],
      parameters: ['test'],
    });
    assert.deepEqual(updates, [
      {
        action: 'remove',
        basedOn: {
          expectations: ['FAIL'],
          parameters: ['test'],
          platforms: ['darwin'],
          testIdPattern: '[page.spec] Page Page.setContent should work',
        },
        expectation: {
          expectations: ['FAIL'],
          parameters: ['test'],
          platforms: ['darwin'],
          testIdPattern: '[page.spec] Page Page.setContent should work',
        },
      },
    ]);
  });