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

Code before:
  jest.resetModules();
});

it('Returns true when running on interactive environment', () => {
  jest.doMock('ci-info', () => ({isCI: false}));
  process.stdout.isTTY = true;
  process.env.TERM = 'xterm-256color';

  const isInteractive = require('../isInteractive').default;
  expect(isInteractive).toBe(true);
});

it('Returns false when running on a non-interactive environment', () => {
  let isInteractive;
  const expectedResult = false;

  // Test with isCI being true and isTTY false
  jest.doMock('ci-info', () => ({isCI: true}));
  process.stdout.isTTY = undefined;
  process.env.TERM = 'xterm-256color';
  isInteractive = require('../isInteractive').default;
  expect(isInteractive).toBe(expectedResult);

  // Test with isCI being false and isTTY false
  jest.resetModules();
  jest.doMock('ci-info', () => ({isCI: false}));
  process.stdout.isTTY = undefined;
  process.env.TERM = 'xterm-256color';
  isInteractive = require('../isInteractive').default;
  expect(isInteractive).toBe(expectedResult);

  // Test with isCI being true and isTTY true
  jest.resetModules();
  jest.doMock('ci-info', () => ({isCI: true}));
  process.stdout.isTTY = true;
  process.env.TERM = 'xterm-256color';
  isInteractive = require('../isInteractive').default;
  expect(isInteractive).toBe(expectedResult);

  // Test with dumb terminal
  jest.resetModules();
  jest.doMock('ci-info', () => ({isCI: false}));
  process.stdout.isTTY = undefined;
  process.env.TERM = 'dumb';
  isInteractive = require('../isInteractive').default;
  expect(isInteractive).toBe(expectedResult);
});


Review comment (suggestion):
To avoid repeating this line, it made sense to employ `each` here.

Code after:
it('Returns true when running in an interactive environment', () => {
  jest.doMock('ci-info', () => ({isCI: false}));
  process.stdout.isTTY = true;
  process.env.TERM = 'xterm-256color';

  const isInteractive = (
    require('../isInteractive') as typeof import('../isInteractive')
  ).default;

  expect(isInteractive).toBe(true);
});

it.each([
  {isCI: false, isTTY: false, term: 'xterm-256color'},
  {isCI: false, isTTY: false, term: 'xterm-256color'},
  {isCI: true, isTTY: true, term: 'xterm-256color'},
  {isCI: false, isTTY: false, term: 'dumb'},
])(
  'Returns false when running in a non-interactive environment',
  ({isCI, isTTY, term}) => {
    jest.doMock('ci-info', () => ({isCI}));
    process.stdout.isTTY = isTTY;
    process.env.TERM = term;

    const isInteractive = (
      require('../isInteractive') as typeof import('../isInteractive')
    ).default;

    expect(isInteractive).toBe(false);
  },
);
Repository: facebook/jest
File: scripts/buildTs.mjs (JavaScript)

Code before:
      jestDependenciesOfPackage,
      `Expected declared references to match dependencies in package ${
        pkg.name
      }. Got:\n\n${references.join(
        '\n',
      )}\nExpected:\n\n${jestDependenciesOfPackage.join('\n')}`,
    );
  }

  let hasJestTestUtils = Object.keys(pkg.dependencies || {}).includes(
    '@jest/test-utils',
  );

  if (hasJestTestUtils) {
    throw new Error(
      chalk.red(
        `Package '${pkg.name}' declares '@jest/test-utils' as dependency, but it must be declared as dev dependency`,
      ),
    );
  }

  hasJestTestUtils = Object.keys(pkg.devDependencies || {}).includes(
    '@jest/test-utils',
  );

  const tsConfigPaths = glob.sync('src/**/tsconfig.json', {
    absolute: true,
    cwd: packageDir,
  });

  const testUtilsReferences = tsConfigPaths.filter(tsConfigPath => {
    const tsConfig = JSON.parse(
      stripJsonComments(fs.readFileSync(tsConfigPath, 'utf8')),
    );
    const references = tsConfig.references.map(({path}) => path);

    return references.some(reference => /test-utils$/.test(reference));
  });

  if (hasJestTestUtils && !testUtilsReferences.length) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' declares '@jest/test-utils' as dev dependency, but it is not referenced in:\n\n${tsConfigPaths.join(
          '\n',
        )}`,
      ),
    );
  }


Review comment (suggestion):
For example, `@jest/core` has several `__tests__` folders. Only some of them include reference to `@jest/test-utils`. The check will pass if any of `__tests__/tsconfig.json` would include the reference.

~~Hm.. perhaps `**/__tests__/tsconfig.json` glob pattern would make more sense?~~ Done.

Code after:
      jestDependenciesOfPackage,
      `Expected declared references to match dependencies in package ${
        pkg.name
      }. Got:\n\n${references.join(
        '\n',
      )}\nExpected:\n\n${jestDependenciesOfPackage.join('\n')}`,
    );
  }

  let hasJestTestUtils = Object.keys(pkg.dependencies || {}).includes(
    '@jest/test-utils',
  );

  if (hasJestTestUtils) {
    throw new Error(
      chalk.red(
        `Package '${pkg.name}' declares '@jest/test-utils' as dependency, but it must be declared as dev dependency`,
      ),
    );
  }

  hasJestTestUtils = Object.keys(pkg.devDependencies || {}).includes(
    '@jest/test-utils',
  );

  const tsConfigPaths = glob.sync('**/__tests__/tsconfig.json', {
    absolute: true,
    cwd: packageDir,
  });

  const testUtilsReferences = tsConfigPaths.filter(tsConfigPath => {
    const tsConfig = JSON.parse(
      stripJsonComments(fs.readFileSync(tsConfigPath, 'utf8')),
    );
    const references = tsConfig.references.map(({path}) => path);

    return references.some(reference => /test-utils$/.test(reference));
  });

  if (hasJestTestUtils && testUtilsReferences.length === 0) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' declares '@jest/test-utils' as dev dependency, but it is not referenced in:\n\n${tsConfigPaths.join(
          '\n',
        )}`,
      ),
    );
  }
Repository: facebook/jest
File: scripts/buildTs.mjs (JavaScript)

Code before:
    throw new Error(
      chalk.red(
        `Package '${pkg.name}' declares '@jest/test-utils' as dependency, but it must be declared as dev dependency`,
      ),
    );
  }

  hasJestTestUtils = Object.keys(pkg.devDependencies || {}).includes(
    '@jest/test-utils',
  );

  const tsConfigPaths = glob.sync('src/**/tsconfig.json', {
    absolute: true,
    cwd: packageDir,
  });

  const testUtilsReferences = tsConfigPaths.filter(tsConfigPath => {
    const tsConfig = JSON.parse(
      stripJsonComments(fs.readFileSync(tsConfigPath, 'utf8')),
    );
    const references = tsConfig.references.map(({path}) => path);

    return references.some(reference => /test-utils$/.test(reference));
  });

  if (hasJestTestUtils && !testUtilsReferences.length) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' declares '@jest/test-utils' as dev dependency, but it is not referenced in:\n\n${tsConfigPaths.join(
          '\n',
        )}`,
      ),
    );
  }

  if (!hasJestTestUtils && testUtilsReferences.length) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' does not declare '@jest/test-utils' as dev dependency, but it is referenced in:\n\n${testUtilsReferences.join(
          '\n',
        )}`,
      ),
    );
  }
});

const args = [

Review comment (suggestion):
```suggestion
  if (hasJestTestUtils && testUtilsReferences.length === 0) {
```

Code after:
    throw new Error(
      chalk.red(
        `Package '${pkg.name}' declares '@jest/test-utils' as dependency, but it must be declared as dev dependency`,
      ),
    );
  }

  hasJestTestUtils = Object.keys(pkg.devDependencies || {}).includes(
    '@jest/test-utils',
  );

  const tsConfigPaths = glob.sync('**/__tests__/tsconfig.json', {
    absolute: true,
    cwd: packageDir,
  });

  const testUtilsReferences = tsConfigPaths.filter(tsConfigPath => {
    const tsConfig = JSON.parse(
      stripJsonComments(fs.readFileSync(tsConfigPath, 'utf8')),
    );
    const references = tsConfig.references.map(({path}) => path);

    return references.some(reference => /test-utils$/.test(reference));
  });

  if (hasJestTestUtils && testUtilsReferences.length === 0) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' declares '@jest/test-utils' as dev dependency, but it is not referenced in:\n\n${tsConfigPaths.join(
          '\n',
        )}`,
      ),
    );
  }

  if (!hasJestTestUtils && testUtilsReferences.length > 0) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' does not declare '@jest/test-utils' as dev dependency, but it is referenced in:\n\n${testUtilsReferences.join(
          '\n',
        )}`,
      ),
    );
  }
});

const args = [
Repository: facebook/jest
File: scripts/buildTs.mjs (JavaScript)

Code before:
    absolute: true,
    cwd: packageDir,
  });

  const testUtilsReferences = tsConfigPaths.filter(tsConfigPath => {
    const tsConfig = JSON.parse(
      stripJsonComments(fs.readFileSync(tsConfigPath, 'utf8')),
    );
    const references = tsConfig.references.map(({path}) => path);

    return references.some(reference => /test-utils$/.test(reference));
  });

  if (hasJestTestUtils && !testUtilsReferences.length) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' declares '@jest/test-utils' as dev dependency, but it is not referenced in:\n\n${tsConfigPaths.join(
          '\n',
        )}`,
      ),
    );
  }

  if (!hasJestTestUtils && testUtilsReferences.length) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' does not declare '@jest/test-utils' as dev dependency, but it is referenced in:\n\n${testUtilsReferences.join(
          '\n',
        )}`,
      ),
    );
  }
});

const args = [
  'tsc',
  '-b',
  ...packagesWithTs.map(({packageDir}) => packageDir),
  ...process.argv.slice(2),
];

console.log(chalk.inverse(' Building TypeScript definition files '));

try {
  await execa('yarn', args, {stdio: 'inherit'});
  console.log(
    chalk.inverse.green(' Successfully built TypeScript definition files '),

Review comment (suggestion):
```suggestion
  if (!hasJestTestUtils && testUtilsReferences.length > 0) {
```

Code after:
    absolute: true,
    cwd: packageDir,
  });

  const testUtilsReferences = tsConfigPaths.filter(tsConfigPath => {
    const tsConfig = JSON.parse(
      stripJsonComments(fs.readFileSync(tsConfigPath, 'utf8')),
    );
    const references = tsConfig.references.map(({path}) => path);

    return references.some(reference => /test-utils$/.test(reference));
  });

  if (hasJestTestUtils && testUtilsReferences.length === 0) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' declares '@jest/test-utils' as dev dependency, but it is not referenced in:\n\n${tsConfigPaths.join(
          '\n',
        )}`,
      ),
    );
  }

  if (!hasJestTestUtils && testUtilsReferences.length > 0) {
    throw new Error(
      chalk.red(
        `Package '${
          pkg.name
        }' does not declare '@jest/test-utils' as dev dependency, but it is referenced in:\n\n${testUtilsReferences.join(
          '\n',
        )}`,
      ),
    );
  }
});

const args = [
  'tsc',
  '-b',
  ...packagesWithTs.map(({packageDir}) => packageDir),
  ...process.argv.slice(2),
];

console.log(chalk.inverse(' Building TypeScript definition files '));

try {
  await execa('yarn', args, {stdio: 'inherit'});
  console.log(
    chalk.inverse.green(' Successfully built TypeScript definition files '),
Repository: facebook/jest
File: packages/jest-test-sequencer/src/index.ts (TypeScript)

Code before:
  }

  _getCache(test: Test): Cache {
    const {context} = test;
    if (!this._cache.has(context) && context.config.cache) {
      const cachePath = this._getCachePath(context);
      if (fs.existsSync(cachePath)) {
        try {
          this._cache.set(
            context,
            JSON.parse(fs.readFileSync(cachePath, 'utf8')) as Cache,
          );
        } catch {}
      }
    }

    let cache = this._cache.get(context);
    if (!cache) {
      cache = {};
      this._cache.set(context, cache);
    }

    return cache;
  }

  _shardPosition(options: ShardOptions & {suiteLength: number}): number {
    const shardRest = options.suiteLength % options.shardCount;
    const ratio = options.suiteLength / options.shardCount;

    return new Array(options.shardIndex)
      .fill(true)
      .reduce((acc, _, shardIndex) => {
        const dangles = shardIndex < shardRest;
        const shardSize = dangles ? Math.ceil(ratio) : Math.floor(ratio);
        return acc + shardSize;
      }, 0);
  }

  /**
   * Select tests for shard requested via --shard=shardIndex/shardCount
   * Sharding is applied before sorting
   *
   * @param tests All tests
   * @param options shardIndex and shardIndex to select
   *
   * @example
   * ```typescript
   * class CustomSequencer extends Sequencer {
   *  shard(tests, { shardIndex, shardCount }) {
   *    const shardSize = Math.ceil(tests.length / options.shardCount);
   *    const shardStart = shardSize * (options.shardIndex - 1);

Review comment (suggestion):
```suggestion
  private _shardPosition(options: ShardOptions & {suiteLength: number}): number {
```

Code after:
    return HasteMapClass.getCacheFilePath(
      config.cacheDirectory,
      `perf-cache-${config.id}`,
    );
  }

  _getCache(test: Test): Cache {
    const {context} = test;
    if (!this._cache.has(context) && context.config.cache) {
      const cachePath = this._getCachePath(context);
      if (fs.existsSync(cachePath)) {
        try {
          this._cache.set(
            context,
            JSON.parse(fs.readFileSync(cachePath, 'utf8')) as Cache,
          );
        } catch {}
      }
    }

    let cache = this._cache.get(context);
    if (!cache) {
      cache = {};
      this._cache.set(context, cache);
    }

    return cache;
  }

  private _shardPosition(options: ShardPositionOptions): number {
    const shardRest = options.suiteLength % options.shardCount;
    const ratio = options.suiteLength / options.shardCount;

    return new Array(options.shardIndex)
      .fill(true)
      .reduce<number>((acc, _, shardIndex) => {
        const dangles = shardIndex < shardRest;
        const shardSize = dangles ? Math.ceil(ratio) : Math.floor(ratio);
        return acc + shardSize;
      }, 0);
  }

  /**
   * Select tests for shard requested via --shard=shardIndex/shardCount
   * Sharding is applied before sorting
   *
   * @param tests All tests
   * @param options shardIndex and shardIndex to select
   *
   * @example
   * ```typescript
Repository: facebook/jest
File: e2e/native-esm/__tests__/native-esm-wasm.test.js (JavaScript)

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.
 */

// the point here is that it's the node core module
// eslint-disable-next-line no-restricted-imports
import {readFileSync} from 'fs';
// The file was generated by wasm-pack
import {getAnswer} from '../42.wasm';

const wasmFileBuffer = readFileSync('42.wasm');

test('supports native wasm imports', () => {
  expect(getAnswer()).toBe(42);
});

test('supports imports from "data:application/wasm" URI with base64 encoding', async () => {
  const importedWasmModule = await import(
    `data:application/wasm;base64,${wasmFileBuffer.toString('base64')}`
  );
  expect(importedWasmModule.getAnswer()).toBe(42);
});

test('imports from "data:text/wasm" URI without explicit encoding fail', async () => {
  await expect(() =>
    import(`data:application/wasm,${wasmFileBuffer.toString('base64')}`),
  ).rejects.toThrow('Missing data URI encoding');
});

test('imports from "data:text/wasm" URI with invalid encoding fail', async () => {
  await expect(() =>
    import('data:application/wasm;charset=utf-8,oops'),
  ).rejects.toThrow('Invalid data URI encoding: charset=utf-8');
});

Review comment (suggestion):
File source: https://github.com/hasharchives/wasm-ts-esm-in-node-jest-and-nextjs

Happy to replace this wasm with some canonical example file, but I don’t know any. Ideally, this file would export a function with arguments (mine is just `getAnswer = () => 42`).

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 {readFileSync} from 'node:fs';
// file origin: https://github.com/mdn/webassembly-examples/blob/2f2163287f86fe29deb162335bccca7d5d95ca4f/understanding-text-format/add.wasm
// source code: https://github.com/mdn/webassembly-examples/blob/2f2163287f86fe29deb162335bccca7d5d95ca4f/understanding-text-format/add.was
import {add} from '../add.wasm';

const wasmFileBuffer = readFileSync('add.wasm');

test('supports native wasm imports', () => {
  expect(add(1, 2)).toBe(3);

  // because arguments are i32 (signed), fractional part is truncated
  expect(add(0.99, 1.01)).toBe(1);

  // because return value is i32 (signed), (2^31 - 1) + 1 overflows and becomes -2^31
  expect(add(Math.pow(2, 31) - 1, 1)).toBe(-Math.pow(2, 31));

  // invalid or missing arguments are treated as 0
  expect(add('hello', 'world')).toBe(0);
  expect(add()).toBe(0);
  expect(add(null)).toBe(0);
  expect(add({}, [])).toBe(0);

  // redundant arguments are silently ignored
  expect(add(1, 2, 3)).toBe(3);
});

test('supports dynamic wasm imports', async () => {
  const {add: dynamicAdd} = await import('../add.wasm');
  expect(dynamicAdd(1, 2)).toBe(3);
});
Repository: facebook/jest
File: packages/jest-mock/src/index.ts (TypeScript)

Code before:

  spyOn<
    T extends object,
    K extends PropertyLikeKeys<T>,
    V extends Required<T>[K],
    A extends 'get' | 'set',
  >(
    object: T,
    methodKey: K,
    accessType: A,
  ): A extends 'get'
    ? SpiedGetter<V>
    : A extends 'set'
    ? SpiedSetter<V>
    : never;

  spyOn<
    T extends object,
    K extends ConstructorLikeKeys<T> | MethodLikeKeys<T>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends ClassLike | FunctionLike ? Spied<V> : never;

  // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types
  spyOn<T extends object, K extends PropertyLikeKeys<T>>(
    object: T,
    methodKey: K,
    accessType?: 'get' | 'set',
  ) {
    if (typeof object !== 'object' && typeof object !== 'function') {
      throw new Error(
        `Cannot spyOn on a primitive value; ${this._typeOf(object)} given`,
      );
    }

    if (!object) {
      throw new Error(
        `spyOn could not find an object to spy upon for ${String(methodKey)}`,
      );
    }

    if (!methodKey) {
      throw new Error('No property name supplied');
    }

    if (accessType) {
      return this._spyOnProperty(object, methodKey, accessType);
    }


Review comment (suggestion):
The overloads are now way more readable. Nice to look at them.

One more detail I am leaving for the future iterations. The return type of `spyOn` and `_spyOnProperty` methods is `MockInstance`, but the value wich is return has to be cast using `as Mock`. This is because of the return type of `_makeComponent` method. It comes from overloads which are causing some type issues. That needs more thinking. So I am leaving this for the future.

Code after:

  spyOn<
    T extends object,
    K extends PropertyLikeKeys<T>,
    V extends Required<T>[K],
    A extends 'get' | 'set',
  >(
    object: T,
    methodKey: K,
    accessType: A,
  ): A extends 'get'
    ? SpiedGetter<V>
    : A extends 'set'
    ? SpiedSetter<V>
    : never;

  spyOn<
    T extends object,
    K extends ConstructorLikeKeys<T> | MethodLikeKeys<T>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends ClassLike | FunctionLike ? Spied<V> : never;

  spyOn<T extends object>(
    object: T,
    methodKey: keyof T,
    accessType?: 'get' | 'set',
  ): MockInstance {
    if (typeof object !== 'object' && typeof object !== 'function') {
      throw new Error(
        `Cannot spyOn on a primitive value; ${this._typeOf(object)} given`,
      );
    }

    if (!object) {
      throw new Error(
        `spyOn could not find an object to spy upon for ${String(methodKey)}`,
      );
    }

    if (!methodKey) {
      throw new Error('No property name supplied');
    }

    if (accessType) {
      return this._spyOnProperty(object, methodKey, accessType);
    }

    const original = object[methodKey];
Repository: facebook/jest
File: e2e/native-esm/__tests__/native-esm-wasm.test.js (JavaScript)

Code before:
test('supports dynamic wasm imports', async () => {
  const {add: dynamicAdd} = await import('../add.wasm');
  expect(dynamicAdd(1, 2)).toBe(3);
});

test('supports imports from "data:application/wasm" URI with base64 encoding', async () => {
  const importedWasmModule = await import(
    `data:application/wasm;base64,${wasmFileBuffer.toString('base64')}`
  );
  expect(importedWasmModule.add(0, 42)).toBe(42);
});

test('imports from "data:application/wasm" URI without explicit encoding fail', async () => {
  await expect(() =>
    import(`data:application/wasm,${wasmFileBuffer.toString('base64')}`),
  ).rejects.toThrow('Missing data URI encoding');
});

test('imports from "data:application/wasm" URI with invalid encoding fail', async () => {
  await expect(() =>
    import('data:application/wasm;charset=utf-8,oops'),
  ).rejects.toThrow('Invalid data URI encoding: charset=utf-8');
});

test('supports wasm files that import js resources (wasm-bindgen)', async () => {
  globalThis.alert = () => {};
  jest.spyOn(globalThis, 'alert');

  const {greet} = await import('../wasm-bindgen/index.js');
  greet('World');

  expect(globalThis.alert).toHaveBeenCalledWith('Hello, World!');
});


Review comment (suggestion):
```suggestion
  globalThis.alert = jest.fn();
```

And delete line below.

(On mobile, so unable to make multi-line suggestions)

Code after:
test('supports dynamic wasm imports', async () => {
  const {add: dynamicAdd} = await import('../add.wasm');
  expect(dynamicAdd(1, 2)).toBe(3);
});

test('supports imports from "data:application/wasm" URI with base64 encoding', async () => {
  const importedWasmModule = await import(
    `data:application/wasm;base64,${wasmFileBuffer.toString('base64')}`
  );
  expect(importedWasmModule.add(0, 42)).toBe(42);
});

test('imports from "data:application/wasm" URI without explicit encoding fail', async () => {
  await expect(() =>
    import(`data:application/wasm,${wasmFileBuffer.toString('base64')}`),
  ).rejects.toThrow('Missing data URI encoding');
});

test('imports from "data:application/wasm" URI with invalid encoding fail', async () => {
  await expect(() =>
    import('data:application/wasm;charset=utf-8,oops'),
  ).rejects.toThrow('Invalid data URI encoding: charset=utf-8');
});

test('supports wasm files that import js resources (wasm-bindgen)', async () => {
  globalThis.alert = jest.fn();

  const {greet} = await import('../wasm-bindgen/index.js');
  greet('World');

  expect(globalThis.alert).toHaveBeenCalledWith('Hello, World!');
});