ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/jest
File: packages/jest-reporters/src/types.ts (TypeScript)

Code before:
  readonly onTestFileStart?: (test: Test) => Promise<void> | void;
  readonly onRunComplete: (
    testContexts: Set<TestContext>,
    results: AggregatedResult,
  ) => Promise<void> | void;
  readonly getLastError: () => Error | void;
}

export type ReporterContext = {
  firstRun: boolean;
  previousSuccess: boolean;
  changedFiles?: Set<string>;
  sourcesRelatedToTestsInChangedFiles?: Set<string>;
  startRun?: (globalConfig: Config.GlobalConfig) => unknown;
};

export type SummaryOptions = {
  currentTestCases?: Array<{test: Test; testCaseResult: TestCaseResult}>;
  estimatedTime?: number;
  roundTime?: boolean;
  width?: number;
  showSeed?: boolean;
  seed?: number;
};

export type SummaryReporterOptions = {
  summaryThreshold?: number;
};


Review comment (suggestion):
Perhaps this type should be moved to `SummaryReporter.ts` file? It is not shared between the modules, so it makes sense to define it near the `SummaryReporter` class.

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

import type {
  AggregatedResult,
  Test,
  TestCaseResult,
  TestContext,
  TestResult,
} from '@jest/test-result';
import type {Config} from '@jest/types';

export type ReporterOnStartOptions = {
  estimatedTime: number;
  showStatus: boolean;
};

export interface Reporter {
  readonly onTestResult?: (
    test: Test,
    testResult: TestResult,
    aggregatedResult: AggregatedResult,
  ) => Promise<void> | void;
  readonly onTestFileResult?: (
    test: Test,
    testResult: TestResult,
    aggregatedResult: AggregatedResult,
  ) => Promise<void> | void;
  readonly onTestCaseResult?: (
    test: Test,
    testCaseResult: TestCaseResult,
  ) => Promise<void> | void;
  readonly onRunStart: (
    results: AggregatedResult,
    options: ReporterOnStartOptions,
  ) => Promise<void> | void;
  readonly onTestStart?: (test: Test) => Promise<void> | void;
  readonly onTestFileStart?: (test: Test) => Promise<void> | void;
  readonly onRunComplete: (
    testContexts: Set<TestContext>,
    results: AggregatedResult,
  ) => Promise<void> | void;
  readonly getLastError: () => Error | void;
}

export type ReporterContext = {
  firstRun: boolean;
  previousSuccess: boolean;
  changedFiles?: Set<string>;
  sourcesRelatedToTestsInChangedFiles?: Set<string>;
  startRun?: (globalConfig: Config.GlobalConfig) => unknown;
};

export type SummaryOptions = {
  currentTestCases?: Array<{test: Test; testCaseResult: TestCaseResult}>;
  estimatedTime?: number;
  roundTime?: boolean;
  width?: number;
  showSeed?: boolean;
  seed?: number;
};
Repository: facebook/jest
File: packages/jest-core/src/TestScheduler.ts (TypeScript)

Code before:
            ? this.addReporter(new VerboseReporter(this._globalConfig))
            : this.addReporter(new DefaultReporter(this._globalConfig));
          break;
        case 'github-actions':
          GITHUB_ACTIONS &&
            this.addReporter(
              new GitHubActionsReporter(this._globalConfig, options),
            );
          break;
        case 'summary':
          summaryOptions = options;
          break;
        default:
          await this._addCustomReporter(reporter, options);
      }
    }

    if (notify) {
      this.addReporter(new NotifyReporter(this._globalConfig, this._context));
    }

    if (coverage) {
      this.addReporter(new CoverageReporter(this._globalConfig, this._context));
    }

    if (summaryOptions) {
      this.addReporter(new SummaryReporter(this._globalConfig, summaryOptions));
    }
  }

  private async _addCustomReporter(
    reporter: string,
    options: Record<string, unknown>,
  ) {
    try {
      const Reporter: ReporterConstructor = await requireOrImportModule(
        reporter,
      );

      this.addReporter(
        new Reporter(this._globalConfig, options, this._context),
      );
    } catch (error: any) {
      error.message = `An error occurred while adding the reporter at path "${chalk.bold(
        reporter,
      )}".\n${error instanceof Error ? error.message : ''}`;
      throw error;
    }
  }

  private async _bailIfNeeded(

Review comment (suggestion):
```suggestion
    if (summaryOptions != null) {
```

Code after:
          verbose
            ? this.addReporter(new VerboseReporter(this._globalConfig))
            : this.addReporter(new DefaultReporter(this._globalConfig));
          break;
        case 'github-actions':
          GITHUB_ACTIONS &&
            this.addReporter(
              new GitHubActionsReporter(this._globalConfig, options),
            );
          break;
        case 'summary':
          summaryOptions = options;
          break;
        default:
          await this._addCustomReporter(reporter, options);
      }
    }

    if (notify) {
      this.addReporter(new NotifyReporter(this._globalConfig, this._context));
    }

    if (coverage) {
      this.addReporter(new CoverageReporter(this._globalConfig, this._context));
    }

    if (summaryOptions != null) {
      this.addReporter(new SummaryReporter(this._globalConfig, summaryOptions));
    }
  }

  private async _addCustomReporter(
    reporter: string,
    options: Record<string, unknown>,
  ) {
    try {
      const Reporter: ReporterConstructor = await requireOrImportModule(
        reporter,
      );

      this.addReporter(
        new Reporter(this._globalConfig, options, this._context),
      );
    } catch (error: any) {
      error.message = `An error occurred while adding the reporter at path "${chalk.bold(
        reporter,
      )}".\n${error instanceof Error ? error.message : ''}`;
      throw error;
    }
  }
Repository: facebook/jest
File: packages/jest-types/src/Config.ts (TypeScript)

Code before:
  showSeed?: boolean;
  shard?: ShardConfig;
  silent?: boolean;
  skipFilter: boolean;
  snapshotFormat: SnapshotFormat;
  errorOnDeprecated: boolean;
  testFailureExitCode: number;
  testNamePattern?: string;
  testPathPattern: string;
  testResultsProcessor?: string;
  testSequencer: string;
  testTimeout?: number;
  updateSnapshot: SnapshotUpdateState;
  useStderr: boolean;
  verbose?: boolean;
  watch: boolean;
  watchAll: boolean;
  watchman: boolean;
  watchPlugins?: Array<{
    path: string;
    config: Record<string, unknown>;
  }> | null;
  workerIdleMemoryLimit?: number;
  workerThreads?: boolean;
};

export type ProjectConfig = {
  automock: boolean;
  cache: boolean;
  cacheDirectory: string;
  clearMocks: boolean;
  coveragePathIgnorePatterns: Array<string>;
  cwd: string;
  dependencyExtractor?: string;
  detectLeaks: boolean;
  detectOpenHandles: boolean;
  displayName?: DisplayName;
  errorOnDeprecated: boolean;
  extensionsToTreatAsEsm: Array<string>;
  fakeTimers: FakeTimers;
  filter?: string;
  forceCoverageMatch: Array<string>;
  globalSetup?: string;
  globalTeardown?: string;
  globals: ConfigGlobals;
  haste: HasteConfig;
  id: string;
  injectGlobals: boolean;
  moduleDirectories: Array<string>;
  moduleFileExtensions: Array<string>;
  moduleNameMapper: Array<[string, string]>;

Review comment (suggestion):
```suggestion
  // TODO: make non-optional in Jest 30
  workerThreads?: boolean;
```

Code after:
  seed: number;
  showSeed?: boolean;
  shard?: ShardConfig;
  silent?: boolean;
  skipFilter: boolean;
  snapshotFormat: SnapshotFormat;
  errorOnDeprecated: boolean;
  testFailureExitCode: number;
  testNamePattern?: string;
  testPathPattern: string;
  testResultsProcessor?: string;
  testSequencer: string;
  testTimeout?: number;
  updateSnapshot: SnapshotUpdateState;
  useStderr: boolean;
  verbose?: boolean;
  watch: boolean;
  watchAll: boolean;
  watchman: boolean;
  watchPlugins?: Array<{
    path: string;
    config: Record<string, unknown>;
  }> | null;
  workerIdleMemoryLimit?: number;
  // TODO: make non-optional in Jest 30
  workerThreads?: boolean;
};

export type ProjectConfig = {
  automock: boolean;
  cache: boolean;
  cacheDirectory: string;
  clearMocks: boolean;
  coveragePathIgnorePatterns: Array<string>;
  cwd: string;
  dependencyExtractor?: string;
  detectLeaks: boolean;
  detectOpenHandles: boolean;
  displayName?: DisplayName;
  errorOnDeprecated: boolean;
  extensionsToTreatAsEsm: Array<string>;
  fakeTimers: FakeTimers;
  filter?: string;
  forceCoverageMatch: Array<string>;
  globalSetup?: string;
  globalTeardown?: string;
  globals: ConfigGlobals;
  haste: HasteConfig;
  id: string;
  injectGlobals: boolean;
  moduleDirectories: Array<string>;
Repository: facebook/jest
File: packages/babel-plugin-jest-hoist/src/__tests__/hoistPlugin.test.ts (TypeScript)

Code before:
      },
      code: formatResult(`
        jest.mock('some-module', () => {
          const actual = jest.requireActual('some-module');

          return jest.fn<typeof actual.method>();
        });
      `),
      formatResult,
      snapshot: true,
    },
    'jest.spyOn call on the imported module': {
      code: formatResult(`
        jest.mock('some-module', () => {
          const module = jest.requireActual('some-module');
          jest.spyOn(module, 'add');
          return module;
        });
      `),
      formatResult,
      snapshot: true,
    },
    'jest.spyOn call in class constructor': {
      code: formatResult(`
        jest.mock('some-module', () => {
          const Actual = jest.requireActual('some-module');
          return class Mocked extends Actual {
            constructor() {
              super();
              jest.spyOn(module, 'add');
            }
          };
        });
      `),
      formatResult,
      snapshot: true,
    },
  },
  /* eslint-enable */
});


Review comment (bug):
```suggestion
          const module = jest.requireActual('some-module');
```

I think? `jest.spyOn(module, 'add');` seems wrong without it.

---

not that it matters, since what we're checking is the hoisting. but still 

Code after:
      },
      code: formatResult(`
        jest.mock('some-module', () => {
          const actual = jest.requireActual('some-module');

          return jest.fn<typeof actual.method>();
        });
      `),
      formatResult,
      snapshot: true,
    },
    'jest.spyOn call on the imported module': {
      code: formatResult(`
        jest.mock('some-module', () => {
          const module = jest.requireActual('some-module');
          jest.spyOn(module, 'add');
          return module;
        });
      `),
      formatResult,
      snapshot: true,
    },
    'jest.spyOn call in class constructor': {
      code: formatResult(`
        jest.mock('some-module', () => {
          const Actual = jest.requireActual('some-module');
          return class Mocked extends Actual {
            constructor() {
              super();
              jest.spyOn(this, 'add');
            }
          };
        });
      `),
      formatResult,
      snapshot: true,
    },
  },
  /* eslint-enable */
});
Repository: facebook/jest
File: packages/jest-config/src/resolveConfigPath.ts (TypeScript)

Code before:

  // This is the system root.
  // We tried everything, config is nowhere to be found ¯\_(ツ)_/¯
  if (pathToResolve === path.dirname(pathToResolve)) {
    throw new Error(makeResolutionErrorMessage(initialPath, cwd));
  }

  // go up a level and try it again
  return resolveConfigPathByTraversing(
    path.dirname(pathToResolve),
    initialPath,
    cwd,
    skipMultipleConfigError,
  );
};

const findPackageJson = (pathToResolve: string) => {
  const packagePath = path.resolve(pathToResolve, PACKAGE_JSON);
  if (isFile(packagePath)) {
    return packagePath;
  }

  return undefined;
};

const resolveJestKey = (packageContent: any, packagePath: string) => {
  if (hasPackageJsonJestKey(packageContent)) {
    const {jest} = JSON.parse(packageContent);

    if (jest && typeof jest === 'string') {
      const resolvedConfigFile = path.resolve(packagePath, jest);

      return isFile(resolvedConfigFile) ? resolvedConfigFile : undefined;
    }
  }

  return undefined;
};

const hasPackageJsonJestKey = (packageContent: any) => {
  try {
    return 'jest' in JSON.parse(packageContent);
  } catch {
    // If package is not a valid JSON
    return false;
  }
};

const makeResolutionErrorMessage = (initialPath: string, cwd: string) =>
  'Could not find a config file based on provided values:\n' +
  `path: "${initialPath}"\n` +

Review comment (suggestion):
```suggestion
const resolveJestKey = (packageContent: string, packagePath: string) => {
```

Code after:
            DOCUMENTATION_NOTE,
          );
        }

        configFiles.push(absolutePath);
      } else {
        configFiles.push(packageJson);
      }
    }
  }

  if (!skipMultipleConfigError && configFiles.length > 1) {
    throw new ValidationError(...makeMultipleConfigsErrorMessage(configFiles));
  }

  if (configFiles.length > 0 || packageJson) {
    return configFiles[0] ?? packageJson;
  }

  // This is the system root.
  // We tried everything, config is nowhere to be found ¯\_(ツ)_/¯
  if (pathToResolve === path.dirname(pathToResolve)) {
    throw new Error(makeResolutionErrorMessage(initialPath, cwd));
  }

  // go up a level and try it again
  return resolveConfigPathByTraversing(
    path.dirname(pathToResolve),
    initialPath,
    cwd,
    skipMultipleConfigError,
  );
};

const findPackageJson = (pathToResolve: string) => {
  const packagePath = path.resolve(pathToResolve, PACKAGE_JSON);
  if (isFile(packagePath)) {
    return packagePath;
  }

  return undefined;
};

const getPackageJsonJestKey = (
  packagePath: string,
): Record<string, unknown> | string | undefined => {
  try {
    const content = fs.readFileSync(packagePath, 'utf8');
    const parsedContent = JSON.parse(content);

    if ('jest' in parsedContent) {
Repository: facebook/jest
File: packages/jest-config/src/resolveConfigPath.ts (TypeScript)

Code before:
  // go up a level and try it again
  return resolveConfigPathByTraversing(
    path.dirname(pathToResolve),
    initialPath,
    cwd,
    skipMultipleConfigError,
  );
};

const findPackageJson = (pathToResolve: string) => {
  const packagePath = path.resolve(pathToResolve, PACKAGE_JSON);
  if (isFile(packagePath)) {
    return packagePath;
  }

  return undefined;
};

const resolveJestKey = (packageContent: any, packagePath: string) => {
  if (hasPackageJsonJestKey(packageContent)) {
    const {jest} = JSON.parse(packageContent);

    if (jest && typeof jest === 'string') {
      const resolvedConfigFile = path.resolve(packagePath, jest);

      return isFile(resolvedConfigFile) ? resolvedConfigFile : undefined;
    }
  }

  return undefined;
};

const hasPackageJsonJestKey = (packageContent: any) => {
  try {
    return 'jest' in JSON.parse(packageContent);
  } catch {
    // If package is not a valid JSON
    return false;
  }
};

const makeResolutionErrorMessage = (initialPath: string, cwd: string) =>
  'Could not find a config file based on provided values:\n' +
  `path: "${initialPath}"\n` +
  `cwd: "${cwd}"\n` +
  'Config paths must be specified by either a direct path to a config\n' +
  'file, or a path to a directory. If directory is given, Jest will try to\n' +
  `traverse directory tree up, until it finds one of those files in exact order: ${JEST_CONFIG_EXT_ORDER.map(
    ext => `"${getConfigFilename(ext)}"`,
  ).join(' or ')}.`;


Review comment (suggestion):
if the path provided isn't a file, we should throw an error

Code after:
      }
    }
  }

  if (!skipMultipleConfigError && configFiles.length > 1) {
    throw new ValidationError(...makeMultipleConfigsErrorMessage(configFiles));
  }

  if (configFiles.length > 0 || packageJson) {
    return configFiles[0] ?? packageJson;
  }

  // This is the system root.
  // We tried everything, config is nowhere to be found ¯\_(ツ)_/¯
  if (pathToResolve === path.dirname(pathToResolve)) {
    throw new Error(makeResolutionErrorMessage(initialPath, cwd));
  }

  // go up a level and try it again
  return resolveConfigPathByTraversing(
    path.dirname(pathToResolve),
    initialPath,
    cwd,
    skipMultipleConfigError,
  );
};

const findPackageJson = (pathToResolve: string) => {
  const packagePath = path.resolve(pathToResolve, PACKAGE_JSON);
  if (isFile(packagePath)) {
    return packagePath;
  }

  return undefined;
};

const getPackageJsonJestKey = (
  packagePath: string,
): Record<string, unknown> | string | undefined => {
  try {
    const content = fs.readFileSync(packagePath, 'utf8');
    const parsedContent = JSON.parse(content);

    if ('jest' in parsedContent) {
      return parsedContent.jest;
    }
  } catch {}
  return undefined;
};

const makeResolutionErrorMessage = (initialPath: string, cwd: string) =>
Repository: facebook/jest
File: packages/jest-config/src/resolveConfigPath.ts (TypeScript)

Code before:
};

const findPackageJson = (pathToResolve: string) => {
  const packagePath = path.resolve(pathToResolve, PACKAGE_JSON);
  if (isFile(packagePath)) {
    return packagePath;
  }

  return undefined;
};

const resolveJestKey = (packageContent: any, packagePath: string) => {
  if (hasPackageJsonJestKey(packageContent)) {
    const {jest} = JSON.parse(packageContent);

    if (jest && typeof jest === 'string') {
      const resolvedConfigFile = path.resolve(packagePath, jest);

      return isFile(resolvedConfigFile) ? resolvedConfigFile : undefined;
    }
  }

  return undefined;
};

const hasPackageJsonJestKey = (packageContent: any) => {
  try {
    return 'jest' in JSON.parse(packageContent);
  } catch {
    // If package is not a valid JSON
    return false;
  }
};

const makeResolutionErrorMessage = (initialPath: string, cwd: string) =>
  'Could not find a config file based on provided values:\n' +
  `path: "${initialPath}"\n` +
  `cwd: "${cwd}"\n` +
  'Config paths must be specified by either a direct path to a config\n' +
  'file, or a path to a directory. If directory is given, Jest will try to\n' +
  `traverse directory tree up, until it finds one of those files in exact order: ${JEST_CONFIG_EXT_ORDER.map(
    ext => `"${getConfigFilename(ext)}"`,
  ).join(' or ')}.`;

function extraIfPackageJson(configPath: string) {
  if (configPath.endsWith(PACKAGE_JSON)) {
    return '`jest` key in ';
  }

  return '';
}

Review comment (suggestion):
```suggestion
const hasPackageJsonJestKey = (packageContent: string) => {
```

Code after:

  if (configFiles.length > 0 || packageJson) {
    return configFiles[0] ?? packageJson;
  }

  // This is the system root.
  // We tried everything, config is nowhere to be found ¯\_(ツ)_/¯
  if (pathToResolve === path.dirname(pathToResolve)) {
    throw new Error(makeResolutionErrorMessage(initialPath, cwd));
  }

  // go up a level and try it again
  return resolveConfigPathByTraversing(
    path.dirname(pathToResolve),
    initialPath,
    cwd,
    skipMultipleConfigError,
  );
};

const findPackageJson = (pathToResolve: string) => {
  const packagePath = path.resolve(pathToResolve, PACKAGE_JSON);
  if (isFile(packagePath)) {
    return packagePath;
  }

  return undefined;
};

const getPackageJsonJestKey = (
  packagePath: string,
): Record<string, unknown> | string | undefined => {
  try {
    const content = fs.readFileSync(packagePath, 'utf8');
    const parsedContent = JSON.parse(content);

    if ('jest' in parsedContent) {
      return parsedContent.jest;
    }
  } catch {}
  return undefined;
};

const makeResolutionErrorMessage = (initialPath: string, cwd: string) =>
  'Could not find a config file based on provided values:\n' +
  `path: "${initialPath}"\n` +
  `cwd: "${cwd}"\n` +
  'Config paths must be specified by either a direct path to a config\n' +
  'file, or a path to a directory. If directory is given, Jest will try to\n' +
  `traverse directory tree up, until it finds one of those files in exact order: ${JEST_CONFIG_EXT_ORDER.map(
    ext => `"${getConfigFilename(ext)}"`,
Repository: facebook/jest
File: packages/jest-resolve/src/resolver.ts (TypeScript)

Code before:
  }

  getModule(name: string): string | null {
    return this._moduleMap.getModule(
      name,
      this._options.defaultPlatform,
      this._supportsNativePlatform,
    );
  }

  getModulePath(from: string, moduleName: string): string {
    if (moduleName[0] !== '.' || path.isAbsolute(moduleName)) {
      return moduleName;
    }
    return path.normalize(`${path.dirname(from)}/${moduleName}`);
  }

  getPackage(name: string): string | null {
    return this._moduleMap.getPackage(
      name,
      this._options.defaultPlatform,
      this._supportsNativePlatform,
    );
  }

  getMockModule(name: string): string | null {
    const mock = this._moduleMap.getMockModule(name);
    return mock ?? null;
  }

  getModulePaths(from: string): Array<string> {
    const cachedModule = this._modulePathCache.get(from);
    if (cachedModule) {
      return cachedModule;
    }

    const moduleDirectory = this._options.moduleDirectories;
    const paths = nodeModulesPaths(from, {moduleDirectory});
    if (paths[paths.length - 1] === undefined) {
      // circumvent node-resolve bug that adds `undefined` as last item.
      paths.pop();
    }
    this._modulePathCache.set(from, paths);
    return paths;
  }

  getGlobalPaths(moduleName?: string): Array<string> {
    if (!moduleName || moduleName[0] === '.' || this.isCoreModule(moduleName)) {
      return [];
    }


Review comment (refactor):
This is a breaking change, right? I mean, Jest packages can be used without Jest. So this and removal of `getMockModuleAsync()` method below are breaking. Looks like good refactor, but has to wait for major release.

Code after:
  }

  getModule(name: string): string | null {
    return this._moduleMap.getModule(
      name,
      this._options.defaultPlatform,
      this._supportsNativePlatform,
    );
  }

  getModulePath(from: string, moduleName: string): string {
    if (moduleName[0] !== '.' || path.isAbsolute(moduleName)) {
      return moduleName;
    }
    return path.normalize(`${path.dirname(from)}/${moduleName}`);
  }

  getPackage(name: string): string | null {
    return this._moduleMap.getPackage(
      name,
      this._options.defaultPlatform,
      this._supportsNativePlatform,
    );
  }

  getMockModule(
    from: string,
    name: string,
    options: Pick<ResolveModuleConfig, 'conditions'>,
  ): string | null {
    const mock = this._moduleMap.getMockModule(name);
    if (mock) {
      return mock;
    } else {
      const resolvedName = this.resolveStubModuleName(from, name, options);
      if (resolvedName) {
        return this._moduleMap.getMockModule(resolvedName) ?? null;
      }
    }
    return null;
  }

  async getMockModuleAsync(
    from: string,
    name: string,
    options: Pick<ResolveModuleConfig, 'conditions'>,
  ): Promise<string | null> {
    const mock = this._moduleMap.getMockModule(name);
    if (mock) {
      return mock;
    } else {