ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/jest
File: packages/jest-mock/src/__tests__/index.test.ts (TypeScript)

Code before:
  test('lastCall gets reset by mockReset', () => {
    const mock = jest.fn();

    mock('first');
    mock('last', 'call');

    expect(mock.mock.lastCall).toEqual(['last', 'call']);

    mock.mockReset();

    expect(mock.mock).not.toHaveProperty('lastCall');
  });

  test('mockName gets reset by mockReset', () => {
    const fn = jest.fn();
    expect(fn.getMockName()).toBe('jest.fn()');
    fn.mockName('myMockFn');
    expect(fn.getMockName()).toBe('myMockFn');
    fn.mockReset();
    expect(fn.getMockName()).toBe('jest.fn()');
  });

  test('after mock reset, the object should return to its original value', () => {
    const myObject = {bar: () => 'bar'};

    const barStub = jest.spyOn(myObject, 'bar');

    barStub.mockReturnValue('POTATO!');
    expect(myObject.bar()).toBe('POTATO!');
    barStub.mockReset();

    expect(myObject.bar()).toBe('bar');
  });

  test('after resetAllMocks, the object should return to its original value', () => {
    const myObject = {bar: () => 'bar'};

    const barStub = jest.spyOn(myObject, 'bar');

    barStub.mockReturnValue('POTATO!');
    expect(myObject.bar()).toBe('POTATO!');
    jest.resetAllMocks();

    expect(myObject.bar()).toBe('bar');
  });

  test('mockName gets reset by mockRestore', () => {
    const fn = jest.fn();
    expect(fn.getMockName()).toBe('jest.fn()');
    fn.mockName('myMockFn');
    expect(fn.getMockName()).toBe('myMockFn');

Review comment (suggestion):
this should use the `moduleMocker` created at the top of this test file, and not the `jest` object

Code after:
  test('lastCall gets reset by mockReset', () => {
    const mock = jest.fn();

    mock('first');
    mock('last', 'call');

    expect(mock.mock.lastCall).toEqual(['last', 'call']);

    mock.mockReset();

    expect(mock.mock).not.toHaveProperty('lastCall');
  });

  test('mockName gets reset by mockReset', () => {
    const fn = jest.fn();
    expect(fn.getMockName()).toBe('jest.fn()');
    fn.mockName('myMockFn');
    expect(fn.getMockName()).toBe('myMockFn');
    fn.mockReset();
    expect(fn.getMockName()).toBe('jest.fn()');
  });

  test('after mock reset, the object should return to its original value', () => {
    const myObject = {bar: () => 'bar'};

    const barStub = moduleMocker.spyOn(myObject, 'bar');

    barStub.mockReturnValue('POTATO!');
    expect(myObject.bar()).toBe('POTATO!');
    barStub.mockReset();

    expect(myObject.bar()).toBe('bar');
  });

  test('after resetAllMocks, the object should return to its original value', () => {
    const myObject = {bar: () => 'bar'};

    const barStub = moduleMocker.spyOn(myObject, 'bar');

    barStub.mockReturnValue('POTATO!');
    expect(myObject.bar()).toBe('POTATO!');
    moduleMocker.resetAllMocks();

    expect(myObject.bar()).toBe('bar');
  });

  test('mockName gets reset by mockRestore', () => {
    const fn = jest.fn();
    expect(fn.getMockName()).toBe('jest.fn()');
    fn.mockName('myMockFn');
    expect(fn.getMockName()).toBe('myMockFn');
Repository: facebook/jest
File: packages/jest-test-result/src/__tests__/formatTestResults.test.ts (TypeScript)

Code before:
/**
 * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import formatTestResults from '../formatTestResults';
import type {AggregatedResult} from '../types';

describe('formatTestResults', () => {
  const assertion = {
    fullName: 'TestedModule#aMethod when some condition is met returns true',
    status: 'passed',
    title: 'returns true',
  };

  const results: AggregatedResult = {
    testResults: [
      {
        numFailingTests: 0,
        perfStats: {end: 2, runtime: 1, slow: false, start: 1},
        // @ts-expect-error
        testResults: [assertion],
      },
    ],
  };

  it('includes test full name', () => {
    const result = formatTestResults(results, undefined, null);
    expect(result.testResults[0].assertionResults[0].fullName).toEqual(
      assertion.fullName,
    );
  });

Review comment (suggestion):
```suggestion
import type {AggregatedResult, AssertionResult} from '../types';
```

Code after:
/**
 * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import formatTestResults from '../formatTestResults';
import type {AggregatedResult, AssertionResult} from '../types';

describe('formatTestResults', () => {
  it('includes test full name', () => {
    const assertion = {
      fullName: 'TestedModule#aMethod when some condition is met returns true',
      status: 'passed',
      title: 'returns true',
    } as AssertionResult;

    const results = {
      testResults: [
        {
          numFailingTests: 0,
          perfStats: {end: 2, runtime: 1, slow: false, start: 1},
          testResults: [assertion],
        },
      ],
    } as AggregatedResult;

    const result = formatTestResults(results, undefined, null);
    expect(result.testResults[0].assertionResults[0].fullName).toEqual(
      assertion.fullName,
    );
  });
Repository: facebook/jest
File: packages/jest-test-result/src/__tests__/formatTestResults.test.ts (TypeScript)

Code before:
      },
    ],
  };

  it('includes test full name', () => {
    const result = formatTestResults(results, undefined, null);
    expect(result.testResults[0].assertionResults[0].fullName).toEqual(
      assertion.fullName,
    );
  });

  const skippedAssertion = {
    fullName: 'Pending test',
    status: 'pending',
    title: 'is still pending',
  };

  const skippedResults: AggregatedResult = {
    testResults: [
      {
        numFailingTests: 0,
        numPassingTests: 0,
        numPendingTests: 2,
        numTodoTests: 2,
        perfStats: {end: 2, runtime: 1, slow: false, start: 1},
        // @ts-expect-error
        testResults: [skippedAssertion],
      },
    ],
  };

  it('should mark result status to skipped', () => {
    const result = formatTestResults(skippedResults, undefined, null);
    expect(result.testResults[0].assertionResults[0].status).toEqual(
      skippedAssertion.status,
    );
  });

  const focusedAssertion = {
    fullName: 'Focused test',
    status: 'focused',
    title: 'focused test',
  };

  const focusedResults: AggregatedResult = {
    testResults: [
      {
        numFailingTests: 0,
        numPassingTests: 1,
        numPendingTests: 1,
        numTodoTests: 2,

Review comment (suggestion):
Instead of `@ts-expect-error` it would be better to use casting (see other suggestions) in all tests of this file.

I see that `@ts-expect-error` was already present here and you just followed the pattern. It works, but casting feels somewhat cleaner.

```suggestion
```

Code after:
        },
      ],
    } as AggregatedResult;

    const result = formatTestResults(results, undefined, null);
    expect(result.testResults[0].assertionResults[0].fullName).toEqual(
      assertion.fullName,
    );
  });

  it('should mark result status to skipped', () => {
    const skippedAssertion = {
      fullName: 'Pending test',
      status: 'pending',
      title: 'is still pending',
    } as AssertionResult;

    const skippedResults = {
      testResults: [
        {
          numFailingTests: 0,
          numPassingTests: 0,
          numPendingTests: 2,
          numTodoTests: 2,
          perfStats: {end: 2, runtime: 1, slow: false, start: 1},
          testResults: [skippedAssertion],
        },
      ],
    } as AggregatedResult;

    const result = formatTestResults(skippedResults, undefined, null);
    expect(result.testResults[0].assertionResults[0].status).toEqual(
      skippedAssertion.status,
    );
  });

  it('should mark result status to focused', () => {
    const focusedAssertion = {
      fullName: 'Focused test',
      status: 'focused',
      title: 'focused test',
    } as AssertionResult;

    const focusedResults = {
      testResults: [
        {
          numFailingTests: 0,
          numPassingTests: 1,
          numPendingTests: 1,
          numTodoTests: 2,
          perfStats: {end: 2, runtime: 1, slow: false, start: 1},
Repository: facebook/jest
File: packages/jest-test-result/src/__tests__/formatTestResults.test.ts (TypeScript)

Code before:
/**
 * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import formatTestResults from '../formatTestResults';
import type {AggregatedResult, AssertionResult} from '../types';

describe('formatTestResults', () => {
  const assertion = {
    fullName: 'TestedModule#aMethod when some condition is met returns true',
    status: 'passed',
    title: 'returns true',
  };

  const results = {
    testResults: [
      {
        numFailingTests: 0,
        perfStats: {end: 2, runtime: 1, slow: false, start: 1},
        testResults: [assertion],
      },
    ],
  } as AggregatedResult;

  it('includes test full name', () => {
    const result = formatTestResults(results, undefined, null);
    expect(result.testResults[0].assertionResults[0].fullName).toEqual(
      assertion.fullName,
    );
  });


Review comment (suggestion):
@SimenB It was unnecessary to add `AssertionResult`. My mistake, see: https://github.com/facebook/jest/pull/13700#discussion_r1059620028

Code after:
/**
 * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import formatTestResults from '../formatTestResults';
import type {AggregatedResult, AssertionResult} from '../types';

describe('formatTestResults', () => {
  it('includes test full name', () => {
    const assertion = {
      fullName: 'TestedModule#aMethod when some condition is met returns true',
      status: 'passed',
      title: 'returns true',
    } as AssertionResult;

    const results = {
      testResults: [
        {
          numFailingTests: 0,
          perfStats: {end: 2, runtime: 1, slow: false, start: 1},
          testResults: [assertion],
        },
      ],
    } as AggregatedResult;

    const result = formatTestResults(results, undefined, null);
    expect(result.testResults[0].assertionResults[0].fullName).toEqual(
      assertion.fullName,
    );
  });
Repository: facebook/jest
File: packages/jest-resolve/src/defaultResolver.ts (TypeScript)

Code before:

    const pkg = readPackageCached(closestPackageJson);

    const resolved = resolveImports(
      {
        base: options.basedir,
        content: pkg,
        path: closestPackageJson,
      },
      path,
      createResolveOptions(options.conditions),
    );

    if (!resolved) {
      throw new Error(
        '`imports` exists, but no results - this is a bug in Jest. Please report an issue',
      );
    }

    const resolvedValues = Array.isArray(resolved) ? resolved : [resolved];

    for (const resolved of resolvedValues) {
      const resolvedPath = resolveByPath(resolved);
      try {
        return resolveSync(resolvedPath, options);
      } catch (e) {
        continue;
      }
    }

    function resolveByPath(resolved: string) {
      if (resolved.startsWith('.')) {
        return pathResolve(dirname(closestPackageJson!), resolved);
      }

      // this is an external module, re-resolve it
      return defaultResolver(resolved, options);
    }
  }

  const segments = path.split('/');

  let moduleName = segments.shift();

  if (moduleName) {
    if (moduleName.startsWith('@')) {
      moduleName = `${moduleName}/${segments.shift()}`;
    }

    // self-reference
    const closestPackageJson = findClosestPackageJson(options.basedir);

Review comment (suggestion):
The build supports `catch { ... }`. I can update this to remove the extra `(e)`

Code after:
      );
    }

    const pkg = readPackageCached(closestPackageJson);

    const resolved = resolveImports(
      {
        base: options.basedir,
        content: pkg,
        path: dirname(closestPackageJson),
      },
      path,
      createImportsResolveOptions(options.conditions),
    );

    if (!resolved) {
      throw new Error(
        '`imports` exists, but no results - this is a bug in Jest. Please report an issue',
      );
    }

    if (resolved.startsWith('.')) {
      return pathResolve(dirname(closestPackageJson), resolved);
    }

    // this is an external module, re-resolve it
    return defaultResolver(resolved, options);
  }

  const segments = path.split('/');

  let moduleName = segments.shift();

  if (moduleName) {
    if (moduleName.startsWith('@')) {
      moduleName = `${moduleName}/${segments.shift()}`;
    }

    // self-reference
    const closestPackageJson = findClosestPackageJson(options.basedir);
    if (closestPackageJson) {
      const pkg = readPackageCached(closestPackageJson);

      if (pkg.name === moduleName && pkg.exports) {
        const subpath = segments.join('/') || '.';

        const resolved = resolveExports(
          pkg,
          subpath,
          createResolveOptions(options.conditions),
        );
Repository: facebook/jest
File: packages/jest-runtime/src/index.ts (TypeScript)

Code before:
      );
    } catch (error: any) {
      this.handleExecutionError(error, module);
    }

    this._isCurrentlyExecutingManualMock = origCurrExecutingManualMock;
    this._currentlyExecutingModulePath = lastExecutingModulePath;
  }

  private transformFile(
    filename: string,
    options?: InternalModuleOptions,
  ): string {
    const source = this.readFile(filename);

    if (options?.isInternalModule) {
      return source;
    }

    const transformedFile = this._scriptTransformer.transform(
      filename,
      this._getFullTransformationOptions(options),
      source,
    );

    if (this._fileTransforms.get(filename)?.code !== transformedFile.code) {
      this._fileTransforms.set(filename, {
        ...transformedFile,
        wrapperLength: this.constructModuleWrapperStart().length,
      });
    }

    if (transformedFile.sourceMapPath) {
      this._sourceMapRegistry.set(filename, transformedFile.sourceMapPath);
    }
    return transformedFile.code;
  }

  private async transformFileAsync(
    filename: string,
    options?: InternalModuleOptions,
  ): Promise<string> {
    const source = this.readFile(filename);

    if (options?.isInternalModule) {
      return source;
    }

    const transformedFile = await this._scriptTransformer.transformAsync(
      filename,
      this._getFullTransformationOptions(options),

Review comment (performance):
I think you can remove this `if` and just always populate the cache. Then it's a single `set` instead of a single `get` and _sometimes_ an additional `set`

Code after:
      );
    } catch (error: any) {
      this.handleExecutionError(error, module);
    }

    this._isCurrentlyExecutingManualMock = origCurrExecutingManualMock;
    this._currentlyExecutingModulePath = lastExecutingModulePath;
  }

  private transformFile(
    filename: string,
    options?: InternalModuleOptions,
  ): string {
    const source = this.readFile(filename);

    if (options?.isInternalModule) {
      return source;
    }

    const transformedFile = this._scriptTransformer.transform(
      filename,
      this._getFullTransformationOptions(options),
      source,
    );

    this._fileTransforms.set(filename, {
      ...transformedFile,
      wrapperLength: this.constructModuleWrapperStart().length,
    });

    if (transformedFile.sourceMapPath) {
      this._sourceMapRegistry.set(filename, transformedFile.sourceMapPath);
    }
    return transformedFile.code;
  }

  private async transformFileAsync(
    filename: string,
    options?: InternalModuleOptions,
  ): Promise<string> {
    const source = this.readFile(filename);

    if (options?.isInternalModule) {
      return source;
    }

    const transformedFile = await this._scriptTransformer.transformAsync(
      filename,
      this._getFullTransformationOptions(options),
      source,
    );
Repository: facebook/jest
File: packages/jest-types/__typetests__/jest.test.ts (TypeScript)

Code before:
expectType<typeof jest>(jest.useFakeTimers({timerLimit: 1000}));
expectError(jest.useFakeTimers({timerLimit: true}));

expectError(jest.useFakeTimers({enableGlobally: true}));
expectError(jest.useFakeTimers('legacy'));
expectError(jest.useFakeTimers('modern'));

expectType<typeof jest>(jest.useRealTimers());
expectError(jest.useRealTimers(true));

// Misc

expectType<typeof jest>(jest.retryTimes(3));
expectType<typeof jest>(jest.retryTimes(3, {logErrorsBeforeRetry: true}));
expectError(jest.retryTimes(3, {logErrorsBeforeRetry: 'all'}));
expectError(jest.retryTimes({logErrorsBeforeRetry: true}));
expectError(jest.retryTimes());

expectType<typeof jest>(jest.setTimeout(6000));
expectError(jest.setTimeout());

expectType<number>(jest.getSeed());
expectError(jest.getSeed(123));

expectType<boolean>(jest.isEnvironmentTornDown());
expectError<boolean>(jest.isEnvironmentTornDown(123));


Review comment (suggestion):
Could be simply:

```suggestion
expectError(jest.isEnvironmentTornDown(123));
```

Code after:
expectType<typeof jest>(jest.useFakeTimers({timerLimit: 1000}));
expectError(jest.useFakeTimers({timerLimit: true}));

expectError(jest.useFakeTimers({enableGlobally: true}));
expectError(jest.useFakeTimers('legacy'));
expectError(jest.useFakeTimers('modern'));

expectType<typeof jest>(jest.useRealTimers());
expectError(jest.useRealTimers(true));

// Misc

expectType<typeof jest>(jest.retryTimes(3));
expectType<typeof jest>(jest.retryTimes(3, {logErrorsBeforeRetry: true}));
expectError(jest.retryTimes(3, {logErrorsBeforeRetry: 'all'}));
expectError(jest.retryTimes({logErrorsBeforeRetry: true}));
expectError(jest.retryTimes());

expectType<typeof jest>(jest.setTimeout(6000));
expectError(jest.setTimeout());

expectType<number>(jest.getSeed());
expectError(jest.getSeed(123));

expectType<boolean>(jest.isEnvironmentTornDown());
expectError(jest.isEnvironmentTornDown(123));
Repository: facebook/jest
File: packages/test-utils/src/ConditionalTest.ts (TypeScript)

Code before:

export function skipSuiteOnJestCircus(): void {
  if (!isJestJasmineRun()) {
    test.only('does not work on jest-circus', () => {
      console.warn('[SKIP] Does not work on jest-circus');
    });
  }
}

export function onNodeVersions(
  versionRange: string,
  testBody: () => void,
): void {
  const description = `on node ${versionRange}`;
  if (semver.satisfies(process.versions.node, versionRange)) {
    describe(description, () => {
      testBody();
    });
  } else {
    describe.skip(description, () => {
      testBody();
    });
  }
}

export function onNotJestJasmine(testBody: () => void): void {
  if (!isJestJasmineRun()) {
    testBody();
  }
}


Review comment (suggestion):
Perhaps `skipTestOnJasmine` would be better name in this case?

```suggestion
export function skipTestOnJasmine(testBody: () => void): void {
```

Code after:
/**
 * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import semver = require('semver');
import type {Global} from '@jest/types';

declare const describe: Global.TestFrameworkGlobals['describe'];
declare const test: Global.TestFrameworkGlobals['test'];

export function isJestJasmineRun(): boolean {
  return process.env.JEST_JASMINE === '1';
}

export function skipSuiteOnJasmine(): void {
  if (isJestJasmineRun()) {
    test.only('does not work on Jasmine', () => {
      console.warn('[SKIP] Does not work on Jasmine');
    });
  }
}

export function skipSuiteOnJestCircus(): void {
  if (!isJestJasmineRun()) {
    test.only('does not work on jest-circus', () => {
      console.warn('[SKIP] Does not work on jest-circus');
    });
  }
}

export function onNodeVersions(
  versionRange: string,
  testBody: () => void,
): void {
  const description = `on node ${versionRange}`;
  if (semver.satisfies(process.versions.node, versionRange)) {
    describe(description, () => {
      testBody();
    });
  } else {
    describe.skip(description, () => {
      testBody();
    });
  }
}
Repository: facebook/jest
File: packages/test-utils/src/ConditionalTest.ts (TypeScript)

Code before:

export function skipSuiteOnJestCircus(): void {
  if (!isJestJasmineRun()) {
    test.only('does not work on jest-circus', () => {
      console.warn('[SKIP] Does not work on jest-circus');
    });
  }
}

export function onNodeVersions(
  versionRange: string,
  testBody: () => void,
): void {
  const description = `on node ${versionRange}`;
  if (semver.satisfies(process.versions.node, versionRange)) {
    describe(description, () => {
      testBody();
    });
  } else {
    describe.skip(description, () => {
      testBody();
    });
  }
}

export function skipTestOnJasmine(testBody: () => void): void {
  if (!isJestJasmineRun()) {
    testBody();
  }
}


Review comment (refactor):
why is a new helper needed? shouldn't `skipSuiteOnJasmine` be enough? That one also supports snapshots properly

Code after:
/**
 * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
 *
 * This source code is licensed under the MIT license found in the
 * LICENSE file in the root directory of this source tree.
 */

import semver = require('semver');
import type {Global} from '@jest/types';

declare const describe: Global.TestFrameworkGlobals['describe'];
declare const test: Global.TestFrameworkGlobals['test'];

export function isJestJasmineRun(): boolean {
  return process.env.JEST_JASMINE === '1';
}

export function skipSuiteOnJasmine(): void {
  if (isJestJasmineRun()) {
    test.only('does not work on Jasmine', () => {
      console.warn('[SKIP] Does not work on Jasmine');
    });
  }
}

export function skipSuiteOnJestCircus(): void {
  if (!isJestJasmineRun()) {
    test.only('does not work on jest-circus', () => {
      console.warn('[SKIP] Does not work on jest-circus');
    });
  }
}

export function onNodeVersions(
  versionRange: string,
  testBody: () => void,
): void {
  const description = `on node ${versionRange}`;
  if (semver.satisfies(process.versions.node, versionRange)) {
    describe(description, () => {
      testBody();
    });
  } else {
    describe.skip(description, () => {
      testBody();
    });
  }
}