ronantakizawa/codereview-bench huggingface dataset:
Repository: facebook/jest
File: packages/jest-mock/__typetests__/utility-types.test.ts (TypeScript)

Code before:

declare const constructorKeys: ConstructorLikeKeys<SomeObject>;

expectType<'SomeClass'>(constructorKeys);

// MethodKeys

declare const classMethods: MethodLikeKeys<SomeClass>;
declare const indexClassMethods: MethodLikeKeys<IndexClass>;
declare const objectMethods: MethodLikeKeys<SomeObject>;
declare const indexObjectMethods: MethodLikeKeys<IndexObject>;

expectType<'methodA' | 'methodB'>(classMethods);
expectType<'methodA' | 'methodB'>(indexClassMethods);
expectType<'methodA' | 'methodB' | 'methodC'>(objectMethods);
expectType<'methodA' | 'methodB' | 'methodC'>(indexObjectMethods);

// PropertyKeys

declare const classProperties: PropertyLikeKeys<SomeClass>;
declare const indexClassProperties: PropertyLikeKeys<IndexClass>;
declare const objectProperties: PropertyLikeKeys<SomeObject>;
declare const indexObjectProperties: PropertyLikeKeys<IndexObject>;

expectType<'propertyA' | 'propertyB' | 'propertyC'>(classProperties);
expectType<string>(indexClassProperties);
expectType<'propertyA' | 'propertyB' | 'someClassInstance'>(objectProperties);
expectType<string>(indexObjectProperties);


Review comment (suggestion):
Fells like `string | number` is more correct or at least more TypeScriptish.

A note from [TS docs](https://www.typescriptlang.org/docs/handbook/2/keyof-types.html#the-keyof-type-operator): `keyof {[k: string]: any}` is of type `string | number`, "because JavaScript object keys are always coerced to a string". 

Code after:

declare const constructorKeys: ConstructorLikeKeys<SomeObject>;

expectType<'SomeClass'>(constructorKeys);

// MethodKeys

declare const classMethods: MethodLikeKeys<SomeClass>;
declare const indexClassMethods: MethodLikeKeys<IndexClass>;
declare const objectMethods: MethodLikeKeys<SomeObject>;
declare const indexObjectMethods: MethodLikeKeys<IndexObject>;

expectType<'methodA' | 'methodB'>(classMethods);
expectType<'methodA' | 'methodB'>(indexClassMethods);
expectType<'methodA' | 'methodB' | 'methodC'>(objectMethods);
expectType<'methodA' | 'methodB' | 'methodC'>(indexObjectMethods);

// PropertyKeys

declare const classProperties: PropertyLikeKeys<SomeClass>;
declare const indexClassProperties: PropertyLikeKeys<IndexClass>;
declare const objectProperties: PropertyLikeKeys<SomeObject>;
declare const indexObjectProperties: PropertyLikeKeys<IndexObject>;

expectType<'propertyA' | 'propertyB' | 'propertyC'>(classProperties);
expectType<string | number>(indexClassProperties);
expectType<'propertyA' | 'propertyB' | 'someClassInstance'>(objectProperties);
expectType<string | number>(indexObjectProperties);
Repository: facebook/jest
File: packages/jest-config/src/__tests__/normalize.test.ts (TypeScript)

Code before:
  it('throws if the options is missing a rootDir property', async () => {
    expect.assertions(1);

    await expect(
      normalize({}, {} as Config.Argv),
    ).rejects.toThrowErrorMatchingSnapshot();
  });
});

describe('automock', () => {
  it('falsy automock is not overwritten', async () => {
    jest.mocked(console.warn).mockImplementation(() => {});
    const {options} = await normalize(
      {
        automock: false,
        rootDir: '/root/path/foo',
      },
      {} as Config.Argv,
    );

    expect(options.automock).toBe(false);
  });
});

describe('collectCoverageOnlyFrom', () => {
  it('normalizes all paths relative to rootDir', async () => {
    const {options} = await normalize(
      {
        collectCoverageOnlyFrom: {
          'bar/baz': true,
          'qux/quux/': true,
        },
        rootDir: '/root/path/foo/',
      },
      {} as Config.Argv,
    );

    const expected = Object.create(null);
    expected[expectedPathFooBar] = true;
    expected[expectedPathFooQux] = true;

    expect(options.collectCoverageOnlyFrom).toEqual(expected);
  });

  it('does not change absolute paths', async () => {
    const {options} = await normalize(
      {
        collectCoverageOnlyFrom: {
          '/an/abs/path': true,
          '/another/abs/path': true,
        },

Review comment (suggestion):
Could not leave it. In this case the tokens are ignored. See the test or code here: https://github.com/facebook/jest/blob/3bef02e5925948b38f035572121a41ec9b633dfa/packages/jest-config/src/normalize.ts#L301-L305

Code after:
  it('throws if the options is missing a rootDir property', async () => {
    expect.assertions(1);

    await expect(
      normalize({}, {} as Config.Argv),
    ).rejects.toThrowErrorMatchingSnapshot();
  });
});

describe('automock', () => {
  it('falsy automock is not overwritten', async () => {
    jest.mocked(console.warn).mockImplementation(() => {});
    const {options} = await normalize(
      {
        automock: false,
        rootDir: '/root/path/foo',
      },
      {} as Config.Argv,
    );

    expect(options.automock).toBe(false);
  });
});

describe('collectCoverageFrom', () => {
  it('ignores <rootDir> tokens', async () => {
    const barBaz = 'bar/baz';
    const quxQuux = 'qux/quux/';
    const notQuxQuux = `!${quxQuux}`;

    const {options} = await normalize(
      {
        collectCoverageFrom: [
          barBaz,
          notQuxQuux,
          `<rootDir>/${barBaz}`,
          `!<rootDir>/${quxQuux}`,
        ],
        rootDir: '/root/path/foo/',
      },
      {} as Config.Argv,
    );

    const expected = [barBaz, notQuxQuux, barBaz, notQuxQuux];

    expect(options.collectCoverageFrom).toEqual(expected);
  });
});

describe('findRelatedTests', () => {
  it('it generates --coverageCoverageFrom patterns when needed', async () => {
Repository: facebook/jest
File: packages/jest-mock/src/index.ts (TypeScript)

Code before:
    const fn = this._makeComponent<T>({
      length,
      type: 'function',
    });
    if (implementation) {
      fn.mockImplementation(implementation);
    }
    return fn;
  }

  spyOn<
    T extends object,
    K extends PropertyLikeKeys<Required<T>>,
    V extends Required<T>[K],
  >(object: T, methodKey: K, accessType: 'get'): SpyInstance<() => V>;

  spyOn<
    T extends object,
    K extends PropertyLikeKeys<Required<T>>,
    V extends Required<T>[K],
  >(object: T, methodKey: K, accessType: 'set'): SpyInstance<(arg: V) => void>;

  spyOn<
    T extends object,
    K extends ConstructorLikeKeys<Required<T>>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends ClassLike
    ? SpyInstance<(...args: ConstructorParameters<V>) => InstanceType<V>>
    : never;

  spyOn<
    T extends object,
    K extends MethodLikeKeys<Required<T>>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends FunctionLike
    ? SpyInstance<(...args: Parameters<V>) => ReturnType<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 (accessType) {

Review comment (refactor):
`V` is just a helper to avoid `SpyInstance<(...args: ConstructorParameters<Required<T>[K]>) => InstanceType<Required<T>[K]>>`

Code after:
    const fn = this._makeComponent<T>({
      length,
      type: 'function',
    });
    if (implementation) {
      fn.mockImplementation(implementation);
    }
    return fn;
  }

  spyOn<
    T extends object,
    K extends PropertyLikeKeys<T>,
    V extends Required<T>[K],
  >(object: T, methodKey: K, accessType: 'get'): SpyInstance<() => V>;

  spyOn<
    T extends object,
    K extends PropertyLikeKeys<T>,
    V extends Required<T>[K],
  >(object: T, methodKey: K, accessType: 'set'): SpyInstance<(arg: V) => void>;

  spyOn<
    T extends object,
    K extends ConstructorLikeKeys<T>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends ClassLike
    ? SpyInstance<(...args: ConstructorParameters<V>) => InstanceType<V>>
    : never;

  spyOn<
    T extends object,
    K extends MethodLikeKeys<T>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends FunctionLike
    ? SpyInstance<(...args: Parameters<V>) => ReturnType<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 (accessType) {
Repository: facebook/jest
File: packages/jest-mock/src/index.ts (TypeScript)

Code before:
    T extends object,
    K extends PropertyLikeKeys<Required<T>>,
    V extends Required<T>[K],
  >(object: T, methodKey: K, accessType: 'get'): SpyInstance<() => V>;

  spyOn<
    T extends object,
    K extends PropertyLikeKeys<Required<T>>,
    V extends Required<T>[K],
  >(object: T, methodKey: K, accessType: 'set'): SpyInstance<(arg: V) => void>;

  spyOn<
    T extends object,
    K extends ConstructorLikeKeys<Required<T>>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends ClassLike
    ? SpyInstance<(...args: ConstructorParameters<V>) => InstanceType<V>>
    : never;

  spyOn<
    T extends object,
    K extends MethodLikeKeys<Required<T>>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends FunctionLike
    ? SpyInstance<(...args: Parameters<V>) => ReturnType<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 (accessType) {
      return this._spyOnProperty(object, methodKey, accessType);
    }

    if (typeof object !== 'object' && typeof object !== 'function') {
      throw new Error(
        `Cannot spyOn on a primitive value; ${this._typeOf(object)} given`,
      );
    }

    const original = object[methodKey];


Review comment (suggestion):
Lines with `Required` do all the magic, because they turn types like `(() => void)) | undefined` into `() => void)`. Something what wasn’t a function, becomes one. This part was not working, otherwise logic is the same.

Code after:
    T extends object,
    K extends PropertyLikeKeys<T>,
    V extends Required<T>[K],
  >(object: T, methodKey: K, accessType: 'get'): SpyInstance<() => V>;

  spyOn<
    T extends object,
    K extends PropertyLikeKeys<T>,
    V extends Required<T>[K],
  >(object: T, methodKey: K, accessType: 'set'): SpyInstance<(arg: V) => void>;

  spyOn<
    T extends object,
    K extends ConstructorLikeKeys<T>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends ClassLike
    ? SpyInstance<(...args: ConstructorParameters<V>) => InstanceType<V>>
    : never;

  spyOn<
    T extends object,
    K extends MethodLikeKeys<T>,
    V extends Required<T>[K],
  >(
    object: T,
    methodKey: K,
  ): V extends FunctionLike
    ? SpyInstance<(...args: Parameters<V>) => ReturnType<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 (accessType) {
      return this._spyOnProperty(object, methodKey, accessType);
    }

    if (typeof object !== 'object' && typeof object !== 'function') {
      throw new Error(
        `Cannot spyOn on a primitive value; ${this._typeOf(object)} given`,
      );
    }

    const original = object[methodKey];
Repository: facebook/jest
File: e2e/__tests__/nativeEsm.test.ts (TypeScript)

Code before:

import {resolve} from 'path';
import {onNodeVersions} from '@jest/test-utils';
import {extractSummary, runYarnInstall} from '../Utils';
import runJest, {getConfig} from '../runJest';

const DIR = resolve(__dirname, '../native-esm');

beforeAll(() => {
  runYarnInstall(DIR);
});

test('test config is without transform', () => {
  const {configs} = getConfig(DIR);

  expect(configs).toHaveLength(1);
  expect(configs[0].transform).toEqual([]);
});

test('runs test with native ESM', () => {
  const {exitCode, stderr, stdout, failed, killed, isCanceled, timedOut} =
    runJest(DIR, ['native-esm.test.js'], {
      nodeOptions: '--experimental-vm-modules --no-warnings',
    });

  expect(failed).toBe(false);
  expect(killed).toBe(false);
  expect(isCanceled).toBe(false);
  expect(timedOut).toBe(false);

  expect(stdout).toBe('');
  expect(exitCode).toBe(0);

  const {summary} = extractSummary(stderr);

  expect(summary).toMatchSnapshot();
});

test('supports top-level await', () => {
  const {exitCode, stderr, stdout} = runJest(DIR, ['native-esm.tla.test.js'], {
    nodeOptions: '--experimental-vm-modules --no-warnings',
  });

  expect(stdout).toBe('');
  expect(exitCode).toBe(0);

  const {summary} = extractSummary(stderr);

  expect(summary).toMatchSnapshot();
});


Review comment (suggestion):
Do an object and `toEqual`? That way all props can be inspected instead of bailing at the first mismatch 

Code after:

import {resolve} from 'path';
import {onNodeVersions} from '@jest/test-utils';
import {extractSummary, runYarnInstall} from '../Utils';
import runJest, {getConfig} from '../runJest';

// for unknown reason the "runs test with native ESM" test occasionally ends up
// with process being killed with SIGSEGV (Segmentation fault) signal
jest.retryTimes(3);

const DIR = resolve(__dirname, '../native-esm');

beforeAll(() => {
  runYarnInstall(DIR);
});

test('test config is without transform', () => {
  const {configs} = getConfig(DIR);

  expect(configs).toHaveLength(1);
  expect(configs[0].transform).toEqual([]);
});

test('runs test with native ESM', () => {
  const {exitCode, stderr, stdout} = runJest(DIR, ['native-esm.test.js'], {
    nodeOptions: '--experimental-vm-modules --no-warnings',
  });

  const {summary} = extractSummary(stderr);

  expect(summary).toMatchSnapshot();
  expect(stdout).toBe('');
  expect(exitCode).toBe(0);
});

test('supports top-level await', () => {
  const {exitCode, stderr, stdout} = runJest(DIR, ['native-esm.tla.test.js'], {
    nodeOptions: '--experimental-vm-modules --no-warnings',
  });

  const {summary} = extractSummary(stderr);

  expect(summary).toMatchSnapshot();
  expect(stdout).toBe('');
  expect(exitCode).toBe(0);
});

// minimum version supported by discord.js
onNodeVersions('>=16.9.0', () => {
  test('support re-exports from CJS of dual packages', () => {
    const {exitCode, stderr, stdout} = runJest(
Repository: facebook/jest
File: packages/expect/src/types.ts (TypeScript)

Code before:
   */
  toHaveReturnedWith<U extends EnsureFunctionLike<T>>(
    expected: ReturnType<U>,
  ): R;
  /**
   * Check that a string matches a regular expression.
   */
  toMatch(expected: string | RegExp): R;
  /**
   * Used to check that a JavaScript object matches a subset of the properties of an object
   */
  toMatchObject(
    expected: Record<string, unknown> | Array<Record<string, unknown>>,
  ): R;
  /**
   * Ensure that a mock function has returned (as opposed to thrown) at least once.
   */
  toReturn(): R;
  /**
   * Ensure that a mock function has returned (as opposed to thrown) a specified number of times.
   */
  toReturnTimes(expected: number): R;
  /**
   * Ensure that a mock function has returned a specified value at least once.
   */
  toReturnWith<U extends EnsureFunctionLike<T>>(expected: ReturnType<U>): R;
  /**
   * Use to test that objects have the same types as well as structure.
   */
  toStrictEqual(expected: unknown): R;
  /**
   * Used to test that a function throws when it is called.
   */
  toThrow(expected?: unknown): R;
  /**
   * If you want to test that a specific error is thrown inside a function.
   */
  toThrowError(expected?: unknown): R;
}


Review comment (suggestion):
Maybe change to this syntax? it works and causes less indirection
```suggestion
  toReturnWith(expected: ReturnType<EnsureFunctionLike<T>>): R;
```

Code after:
   */
  toMatch(expected: string | RegExp): R;
  /**
   * Used to check that a JavaScript object matches a subset of the properties of an object
   */
  toMatchObject(
    expected: Record<string, unknown> | Array<Record<string, unknown>>,
  ): R;
  /**
   * Ensure that a mock function has returned (as opposed to thrown) at least once.
   */
  toReturn(): R;
  /**
   * Ensure that a mock function has returned (as opposed to thrown) a specified number of times.
   */
  toReturnTimes(expected: number): R;
  /**
   * Ensure that a mock function has returned a specified value at least once.
   */
  toReturnWith(expected: ReturnType<EnsureFunctionLike<T>>): R;
  /**
   * Use to test that objects have the same types as well as structure.
   */
  toStrictEqual(expected: unknown): R;
  /**
   * Used to test that a function throws when it is called.
   */
  toThrow(expected?: unknown): R;
  /**
   * If you want to test that a specific error is thrown inside a function.
   */
  toThrowError(expected?: unknown): R;
}
Repository: facebook/jest
File: packages/jest-mock/src/__tests__/index.test.ts (TypeScript)

Code before:
      );

      testFn();
      expect(mock1).toHaveBeenCalled();
    });

    it('returns a promise if the provided callback is asynchronous', async () => {
      const mock1 = jest.fn();
      const mock2 = jest.fn();

      const Module = jest.fn(() => ({someFn: mock1}));
      const testFn = function () {
        const m = new Module();
        m.someFn();
      };

      const promise = Module.withImplementation(
        () => ({someFn: mock2}),
        async () => {
          testFn();
          expect(mock2).toHaveBeenCalled();
          expect(mock1).not.toHaveBeenCalled();
        },
      );

      // Is there a better way to detect a promise?
      expect(typeof promise.then).toBe('function');

      await promise;

      testFn();
      expect(mock1).toHaveBeenCalled();
    });
  });

  test('mockReturnValue does not override mockImplementationOnce', () => {
    const mockFn = jest
      .fn()
      .mockReturnValue(1)
      .mockImplementationOnce(() => 2);
    expect(mockFn()).toBe(2);
    expect(mockFn()).toBe(1);
  });

  test('mockImplementation resets the mock', () => {
    const fn = jest.fn();
    expect(fn()).toBeUndefined();
    fn.mockReturnValue('returnValue');
    fn.mockImplementation(() => 'foo');
    expect(fn()).toBe('foo');
  });

Review comment (suggestion):
Perhaps Node’s build-in [util.types.isPromise](https://nodejs.org/api/util.html#utiltypesispromisevalue)? Edit: I think it is fine how you did it.

Code after:
        },
      );

      testFn();
      expect(mock1).toHaveBeenCalled();

      expect.assertions(3);
    });

    it('returns a promise if the provided callback is asynchronous', async () => {
      const mock1 = jest.fn();
      const mock2 = jest.fn();

      const Module = jest.fn(() => ({someFn: mock1}));
      const testFn = function () {
        const m = new Module();
        m.someFn();
      };

      const promise = Module.withImplementation(
        () => ({someFn: mock2}),
        async () => {
          testFn();
          expect(mock2).toHaveBeenCalled();
          expect(mock1).not.toHaveBeenCalled();
        },
      );

      expect(util.types.isPromise(promise)).toBe(true);

      await promise;

      testFn();
      expect(mock1).toHaveBeenCalled();

      expect.assertions(4);
    });
  });

  test('mockReturnValue does not override mockImplementationOnce', () => {
    const mockFn = jest
      .fn()
      .mockReturnValue(1)
      .mockImplementationOnce(() => 2);
    expect(mockFn()).toBe(2);
    expect(mockFn()).toBe(1);
  });

  test('mockImplementation resets the mock', () => {
    const fn = jest.fn();
    expect(fn()).toBeUndefined();
Repository: facebook/jest
File: packages/jest-mock/__typetests__/mock-functions.test.ts (TypeScript)

Code before:
);
expectError(fn(() => Promise.resolve('')).mockResolvedValueOnce(123));
expectError(fn(() => Promise.resolve('')).mockResolvedValueOnce());

expectType<Mock<() => Promise<string>>>(
  fn(() => Promise.resolve('')).mockRejectedValue(new Error('Mock error')),
);
expectType<Mock<() => Promise<string>>>(
  fn(() => Promise.resolve('')).mockRejectedValue('Mock error'),
);
expectError(fn(() => Promise.resolve('')).mockRejectedValue());

expectType<Mock<() => Promise<string>>>(
  fn(() => Promise.resolve('')).mockRejectedValueOnce(new Error('Mock error')),
);
expectType<Mock<() => Promise<string>>>(
  fn(() => Promise.resolve('')).mockRejectedValueOnce('Mock error'),
);
expectError(fn(() => Promise.resolve('')).mockRejectedValueOnce());

expectType<void>(mockFn.withImplementation(mockFnImpl, () => {}));
expectType<Promise<void>>(
  mockFn.withImplementation(mockFnImpl, async () => {}),
);

expectError(mockFn.withImplementation(mockFnImpl, () => {}).then());

const spiedArray = ['a', 'b'];

const spiedFunction = () => {};

const spiedObject = {
  _propertyB: false,

  methodA() {
    return true;
  },
  methodB(a: string, b: number) {
    return;
  },
  methodC(e: any) {
    throw new Error();
  },

  propertyA: 'abc',

  set propertyB(value) {
    this._propertyB = value;
  },
  get propertyB() {
    return this._propertyB;

Review comment (suggestion):
Just above you show that it returns `void`, so maybe it is better to check that it requires two args?

Also leave the comment, please. I need it (;

```suggestion
expectError(mockFn.withImplementation(mockFnImpl);

// jest.spyOn()
```

Code after:
);
expectError(fn(() => Promise.resolve('')).mockResolvedValueOnce(123));
expectError(fn(() => Promise.resolve('')).mockResolvedValueOnce());

expectType<Mock<() => Promise<string>>>(
  fn(() => Promise.resolve('')).mockRejectedValue(new Error('Mock error')),
);
expectType<Mock<() => Promise<string>>>(
  fn(() => Promise.resolve('')).mockRejectedValue('Mock error'),
);
expectError(fn(() => Promise.resolve('')).mockRejectedValue());

expectType<Mock<() => Promise<string>>>(
  fn(() => Promise.resolve('')).mockRejectedValueOnce(new Error('Mock error')),
);
expectType<Mock<() => Promise<string>>>(
  fn(() => Promise.resolve('')).mockRejectedValueOnce('Mock error'),
);
expectError(fn(() => Promise.resolve('')).mockRejectedValueOnce());

expectType<void>(mockFn.withImplementation(mockFnImpl, () => {}));
expectType<Promise<void>>(
  mockFn.withImplementation(mockFnImpl, async () => {}),
);

expectError(mockFn.withImplementation(mockFnImpl));

// jest.spyOn()

const spiedArray = ['a', 'b'];

const spiedFunction = () => {};

const spiedObject = {
  _propertyB: false,

  methodA() {
    return true;
  },
  methodB(a: string, b: number) {
    return;
  },
  methodC(e: any) {
    throw new Error();
  },

  propertyA: 'abc',

  set propertyB(value) {
    this._propertyB = value;
  },