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

Code before:
      global: global as unknown as typeof globalThis,
    });
  }

  // eslint-disable-next-line @typescript-eslint/no-empty-function
  async setup(): Promise<void> {}

  async teardown(): Promise<void> {
    if (this.fakeTimers) {
      this.fakeTimers.dispose();
    }
    if (this.fakeTimersModern) {
      this.fakeTimersModern.dispose();
    }
    if (this.global != null) {
      if (this.errorEventListener) {
        this.global.removeEventListener('error', this.errorEventListener);
      }
      this.global.close();
    }
    this.errorEventListener = null;
    // @ts-expect-error: this.global not allowed to be `null`
    this.global = null;
    this.dom = null;
    this.fakeTimers = null;
    this.fakeTimersModern = null;
  }

  exportConditions(): Array<string> {
    return this.customExportConditions;
  }

  getVmContext(): Context | null {
    if (this.dom) {
      return this.dom.getInternalVMContext();
    }
    return null;
  }
}

export const TestEnvironment = JSDOMEnvironment;


Review comment (suggestion):
added in #5955 - @mjesun you don't by any chance remember what it was added for? `this.global.document` is `undefined` after calling `this.global.close();`

Code after:
      config: projectConfig,
      global: global as unknown as typeof globalThis,
    });
  }

  // eslint-disable-next-line @typescript-eslint/no-empty-function
  async setup(): Promise<void> {}

  async teardown(): Promise<void> {
    if (this.fakeTimers) {
      this.fakeTimers.dispose();
    }
    if (this.fakeTimersModern) {
      this.fakeTimersModern.dispose();
    }
    if (this.global != null) {
      if (this.errorEventListener) {
        this.global.removeEventListener('error', this.errorEventListener);
      }
      this.global.close();
    }
    this.errorEventListener = null;
    // @ts-expect-error: this.global not allowed to be `null`
    this.global = null;
    this.dom = null;
    this.fakeTimers = null;
    this.fakeTimersModern = null;
  }

  exportConditions(): Array<string> {
    return this._configuredExportConditions ?? this.customExportConditions;
  }

  getVmContext(): Context | null {
    if (this.dom) {
      return this.dom.getInternalVMContext();
    }
    return null;
  }
}

export const TestEnvironment = JSDOMEnvironment;
Repository: facebook/jest
File: packages/jest-create-cache-key-function/src/index.ts (TypeScript)

Code before:
  return (sourceText, sourcePath, configString, options) => {
    // Jest 27 passes a single options bag which contains `configString` rather than as a separate argument.
    // We can hide that API difference, though, so this module is usable for both jest@<27 and jest@>=27
    const inferredOptions = options || configString;
    const {config, instrument} = inferredOptions;

    return createHash('sha1')
      .update(globalCacheKey)
      .update('\0', 'utf8')
      .update(sourceText)
      .update('\0', 'utf8')
      .update(config.rootDir ? relative(config.rootDir, sourcePath) : '')
      .update('\0', 'utf8')
      .update(instrument ? 'instrument' : '')
      .digest('hex')
      .substring(0, 32);
  };
}

/**
 * Computes the cache key given  a collection of files and values.  This limits
 * the output with a provided length.
 * @param files list of files to read
 * @param values list of values to add to the computation
 * @param length length of the resulting key defaults to 16 on win32 and 32 elsewhere
 * @returns {string} the global cache key
 */
export default function createCacheKey(
  files: Array<string> = [],
  values: Array<string> = [],
  length?: number,
): GetCacheKeyFunction {
  const theLength = length ?? (process.platform === 'win32' ? 16 : 32);
  return getCacheKeyFunction(getGlobalCacheKey(files, values, theLength));
}


Review comment (performance):
Why the return type on this function is declared `GetCacheKeyFunction`?

Code after:

    return createHash('sha1')
      .update(globalCacheKey)
      .update('\0', 'utf8')
      .update(sourceText)
      .update('\0', 'utf8')
      .update(config.rootDir ? relative(config.rootDir, sourcePath) : '')
      .update('\0', 'utf8')
      .update(instrument ? 'instrument' : '')
      .digest('hex')
      .substring(0, length);
  };
}

/**
 * Returns a function that can be used to generate cache keys based on source code of provided files and provided values.
 *
 * @param files - Array of absolute paths to files whose code should be accounted for when generating cache key
 * @param values - Array of string values that should be accounted for when generating cache key
 * @param length - Length of the resulting key. The default is `32`, or `16` on Windows.
 * @returns A function that can be used to generate cache keys.
 */
export default function createCacheKey(
  files: Array<string> = [],
  values: Array<string> = [],
  length = process.platform === 'win32' ? 16 : 32,
): GetCacheKeyFunction {
  return getCacheKeyFunction(getGlobalCacheKey(files, values, length), length);
}
Repository: facebook/jest
File: packages/jest-create-cache-key-function/src/index.ts (TypeScript)

Code before:
    .substring(0, length);
}

function getCacheKeyFunction(globalCacheKey: string): GetCacheKeyFunction {
  return (sourceText, sourcePath, configString, options) => {
    // Jest 27 passes a single options bag which contains `configString` rather than as a separate argument.
    // We can hide that API difference, though, so this module is usable for both jest@<27 and jest@>=27
    const inferredOptions = options || configString;
    const {config, instrument} = inferredOptions;

    return createHash('sha1')
      .update(globalCacheKey)
      .update('\0', 'utf8')
      .update(sourceText)
      .update('\0', 'utf8')
      .update(config.rootDir ? relative(config.rootDir, sourcePath) : '')
      .update('\0', 'utf8')
      .update(instrument ? 'instrument' : '')
      .digest('hex')
      .substring(0, 32);
  };
}

/**
 * Computes the cache key given  a collection of files and values.  This limits
 * the output with a provided length.
 * @param files list of files to read
 * @param values list of values to add to the computation
 * @param length length of the resulting key defaults to 16 on win32 and 32 elsewhere
 * @returns {string} the global cache key
 */
export default function createCacheKey(
  files: Array<string> = [],
  values: Array<string> = [],
  length?: number,
): GetCacheKeyFunction {
  const theLength = length ?? (process.platform === 'win32' ? 16 : 32);
  return getCacheKeyFunction(getGlobalCacheKey(files, values, theLength));
}


Review comment (performance):
Avoid double spaces and keep it shorter.

```suggestion
 * Computes the cache key given a collection of files and values.
 *
```

Code after:
    // Jest 27 passes a single options bag which contains `configString` rather than as a separate argument.
    // We can hide that API difference, though, so this module is usable for both jest@<27 and jest@>=27
    const inferredOptions = options || configString;
    const {config, instrument} = inferredOptions;

    return createHash('sha1')
      .update(globalCacheKey)
      .update('\0', 'utf8')
      .update(sourceText)
      .update('\0', 'utf8')
      .update(config.rootDir ? relative(config.rootDir, sourcePath) : '')
      .update('\0', 'utf8')
      .update(instrument ? 'instrument' : '')
      .digest('hex')
      .substring(0, length);
  };
}

/**
 * Returns a function that can be used to generate cache keys based on source code of provided files and provided values.
 *
 * @param files - Array of absolute paths to files whose code should be accounted for when generating cache key
 * @param values - Array of string values that should be accounted for when generating cache key
 * @param length - Length of the resulting key. The default is `32`, or `16` on Windows.
 * @returns A function that can be used to generate cache keys.
 */
export default function createCacheKey(
  files: Array<string> = [],
  values: Array<string> = [],
  length = process.platform === 'win32' ? 16 : 32,
): GetCacheKeyFunction {
  return getCacheKeyFunction(getGlobalCacheKey(files, values, length), length);
}
Repository: facebook/jest
File: packages/jest-create-cache-key-function/src/__tests__/index.test.ts (TypeScript)

Code before:
    instrument: false,
  });
  const hashC = createCacheKey('test', 'test.js', null, {
    config: {},
    instrument: true,
  });

  expect(hashA).toHaveLength(32);
  expect(hashA).not.toEqual(hashB);
  expect(hashA).not.toEqual(hashC);
});

test('creation of a cache key on win32', () => {
  process.platform = 'win32';
  const createCacheKeyFunction = interopRequireDefault(
    require('../index'),
  ).default;
  const createCacheKey = createCacheKeyFunction([], ['value']);
  const hashA = createCacheKey('test', 'test.js', null, {
    config: {},
    instrument: false,
  });
  const hashB = createCacheKey('test code;', 'test.js', null, {
    config: {},
    instrument: false,
  });
  const hashC = createCacheKey('test', 'test.js', null, {
    config: {},
    instrument: true,
  });

  expect(hashA).toHaveLength(16);
  expect(hashA).not.toEqual(hashB);
  expect(hashA).not.toEqual(hashC);
});


Review comment (refactor):
Thanks. I think you can simplify the test and just check the length. The rest is tested above.

Would be great to add another test for custom length of the key.

Code after:
    instrument: false,
  });
  const hashC = createCacheKey('test', 'test.js', null, {
    config: {},
    instrument: true,
  });

  expect(hashA).toHaveLength(32);
  expect(hashA).not.toEqual(hashB);
  expect(hashA).not.toEqual(hashC);
});

test('creation of a cache key on win32', () => {
  Object.defineProperty(process, 'platform', {
    value: 'win32',
  });
  const createCacheKeyFunction = interopRequireDefault(
    require('../index'),
  ).default;
  const createCacheKey = createCacheKeyFunction([], ['value']);
  const hashA = createCacheKey('test', 'test.js', null, {
    config: {},
    instrument: false,
  });

  expect(hashA).toHaveLength(16);
});
Repository: facebook/jest
File: packages/jest-core/src/cli/index.ts (TypeScript)

Code before:
        // Expect an undefined return value unless there was an error.
        const err = await filterSetupPromise;
        if (err) {
          throw err;
        }
      }
      return rawFilter(testPaths);
    };
  }

  const {contexts, hasteMapInstances} = await buildContextsAndHasteMaps(
    configs,
    globalConfig,
    outputStream,
  );

  globalConfig.watch || globalConfig.watchAll
    ? await runWatch(
        contexts,
        configs,
        hasDeprecationWarnings,
        globalConfig,
        outputStream,
        hasteMapInstances,
        filter,
      )
    : await runWithoutWatch(
        globalConfig,
        contexts,
        outputStream,
        onComplete,
        changedFilesPromise,
        filter,
      );
};

const runWatch = async (
  contexts: Array<TestContext>,
  _configs: Array<Config.ProjectConfig>,
  hasDeprecationWarnings: boolean,
  globalConfig: Config.GlobalConfig,
  outputStream: NodeJS.WriteStream,
  hasteMapInstances: Array<IHasteMap>,
  filter?: Filter,
) => {
  if (hasDeprecationWarnings) {
    try {
      await handleDeprecationWarnings(outputStream, process.stdin);
      return await watch(
        globalConfig,
        contexts,

Review comment (performance):
would using something like https://nodejs.org/api/perf_hooks.html#performancetimerifyfn-options make sense? both here and other places with a `start` & `end` pair

Code after:
          await rawFilter.setup();
        } catch (err) {
          return err;
        }
        return undefined;
      })();
    }
    filter = async (testPaths: Array<string>) => {
      if (filterSetupPromise) {
        // Expect an undefined return value unless there was an error.
        const err = await filterSetupPromise;
        if (err) {
          throw err;
        }
      }
      return rawFilter(testPaths);
    };
  }

  performance.mark('jest/buildContextsAndHasteMaps:start');
  const {contexts, hasteMapInstances} = await buildContextsAndHasteMaps(
    configs,
    globalConfig,
    outputStream,
  );
  performance.mark('jest/buildContextsAndHasteMaps:end');

  globalConfig.watch || globalConfig.watchAll
    ? await runWatch(
        contexts,
        configs,
        hasDeprecationWarnings,
        globalConfig,
        outputStream,
        hasteMapInstances,
        filter,
      )
    : await runWithoutWatch(
        globalConfig,
        contexts,
        outputStream,
        onComplete,
        changedFilesPromise,
        filter,
      );
};

const runWatch = async (
  contexts: Array<TestContext>,
  _configs: Array<Config.ProjectConfig>,
  hasDeprecationWarnings: boolean,
Repository: facebook/jest
File: packages/jest-mock/src/index.ts (TypeScript)

Code before:
      },
    };

    restore.object = object;
    restore.property = propertyKey;
    restore.replaced = replaced;

    this._spyState.add({restore});

    return replaced.replaceValue(value);
  }

  clearAllMocks(): void {
    this._mockState = new WeakMap();
  }

  resetAllMocks(): void {
    this.clearAllMocks();
    this._mockConfigRegistry = new WeakMap();
    this._spyState.forEach(spyState => spyState.reset?.());
  }

  restoreAllMocks(): void {
    this.clearAllMocks();
    this._mockConfigRegistry = new WeakMap();
    this._spyState.forEach(spyState => spyState.restore());
    this._spyState = new Set();
  }

  private _typeOf(value: unknown): string {
    return value == null ? `${value}` : typeof value;
  }

  mocked<T extends object>(source: T, options?: {shallow: false}): Mocked<T>;
  mocked<T extends object>(
    source: T,
    options: {shallow: true},
  ): MockedShallow<T>;
  mocked<T extends object>(
    source: T,
    _options?: {shallow: boolean},
  ): Mocked<T> | MockedShallow<T> {
    return source as Mocked<T> | MockedShallow<T>;
  }
}

const JestMock = new ModuleMocker(globalThis);

export const fn = JestMock.fn.bind(JestMock);
export const spyOn = JestMock.spyOn.bind(JestMock);
export const mocked = JestMock.mocked.bind(JestMock);

Review comment (suggestion):
Other idea was to call `this.resetAllMocks()` here instead of `this.clearAllMocks()`. Looks like resetting implementation in this case is just waste of time, because that implementation will be discarded in next lines.

Code after:

        this._spyState.delete({restore});
      },
    };

    restore.object = object;
    restore.property = propertyKey;
    restore.replaced = replaced;

    this._spyState.add({restore});

    return replaced.replaceValue(value);
  }

  clearAllMocks(): void {
    this._mockState = new WeakMap();
  }

  resetAllMocks(): void {
    this.clearAllMocks();
    this._mockConfigRegistry = new WeakMap();
    this._spyState.forEach(spyState => spyState.reset?.());
  }

  restoreAllMocks(): void {
    this.clearAllMocks();
    this._mockConfigRegistry = new WeakMap();
    this._spyState.forEach(spyState => spyState.restore());
    this._spyState = new Set();
  }

  private _typeOf(value: unknown): string {
    return value == null ? `${value}` : typeof value;
  }

  mocked<T extends object>(source: T, options?: {shallow: false}): Mocked<T>;
  mocked<T extends object>(
    source: T,
    options: {shallow: true},
  ): MockedShallow<T>;
  mocked<T extends object>(
    source: T,
    _options?: {shallow: boolean},
  ): Mocked<T> | MockedShallow<T> {
    return source as Mocked<T> | MockedShallow<T>;
  }
}

const JestMock = new ModuleMocker(globalThis);

export const fn = JestMock.fn.bind(JestMock);
Repository: facebook/jest
File: packages/jest-message-util/src/__tests__/messages.test.ts (TypeScript)

Code before:
    stack = ['  at stack (file:///C:/Users/user/project/inline.mjs:1:1)'];
    expectedFile = 'C:/Users/user/project/inline.mjs';
  } else {
    stack = ['  at stack (file:///Users/user/project/inline.mjs:1:1)'];
    expectedFile = '/Users/user/project/inline.mjs';
  }
  const frame = getTopFrame(stack);

  expect(frame!.file).toBe(expectedFile);
});

it('should return the error cause if there is one', () => {
  const error = new Error('Test exception');
  // We need to do it this way because in older Javascript engines the constructor does not allow supplying a cause.
  error.cause = new Error('Cause Error');
  const message = formatExecError(
    error,
    {
      rootDir: '',
      testMatch: [],
    },
    {
      noStackTrace: false,
    },
  );
  expect(message).toEqual(expect.stringContaining('Cause'));
});


Review comment (suggestion):
Perhaps it makes sense to snapshot it:

```suggestion
  expect(message).toMatchSnapshot();
```

Code after:
    stack = ['  at stack (file:///C:/Users/user/project/inline.mjs:1:1)'];
    expectedFile = 'C:/Users/user/project/inline.mjs';
  } else {
    stack = ['  at stack (file:///Users/user/project/inline.mjs:1:1)'];
    expectedFile = '/Users/user/project/inline.mjs';
  }
  const frame = getTopFrame(stack);

  expect(frame!.file).toBe(expectedFile);
});

it('should return the error cause if there is one', () => {
  const error = new Error('Test exception');
  // TODO pass `cause` to the `Error` constructor when lowest supported Node version is 16.9.0 and above
  // See https://github.com/nodejs/node/blob/main/doc/changelogs/CHANGELOG_V16.md#error-cause
  error.cause = new Error('Cause Error');
  const message = formatExecError(
    error,
    {
      rootDir: '',
      testMatch: [],
    },
    {
      noStackTrace: false,
    },
  );
  expect(message).toMatchSnapshot();
});
Repository: facebook/jest
File: packages/jest-message-util/src/__tests__/messages.test.ts (TypeScript)

Code before:
      },
      'path_test',
    );

    expect(message).toMatchSnapshot();
  });
});

it('getTopFrame should return a path for mjs files', () => {
  let stack: Array<string>;
  let expectedFile: string;
  if (process.platform === 'win32') {
    stack = ['  at stack (file:///C:/Users/user/project/inline.mjs:1:1)'];
    expectedFile = 'C:/Users/user/project/inline.mjs';
  } else {
    stack = ['  at stack (file:///Users/user/project/inline.mjs:1:1)'];
    expectedFile = '/Users/user/project/inline.mjs';
  }
  const frame = getTopFrame(stack);

  expect(frame!.file).toBe(expectedFile);
});

it('should return the error cause if there is one', () => {
  const error = new Error('Test exception');
  // We need to do it this way because in older Javascript engines the constructor does not allow supplying a cause.
  error.cause = new Error('Cause Error');
  const message = formatExecError(
    error,
    {
      rootDir: '',
      testMatch: [],
    },
    {
      noStackTrace: false,
    },
  );
  expect(message).toEqual(expect.stringContaining('Cause'));
});


Review comment (suggestion):
Please double check version of Node.

```suggestion
  // TODO pass `cause` to the `Error` constructor when lowest supported Node version is 16.9 and above
```

Code after:
      },
      'path_test',
    );

    expect(message).toMatchSnapshot();
  });
});

it('getTopFrame should return a path for mjs files', () => {
  let stack: Array<string>;
  let expectedFile: string;
  if (process.platform === 'win32') {
    stack = ['  at stack (file:///C:/Users/user/project/inline.mjs:1:1)'];
    expectedFile = 'C:/Users/user/project/inline.mjs';
  } else {
    stack = ['  at stack (file:///Users/user/project/inline.mjs:1:1)'];
    expectedFile = '/Users/user/project/inline.mjs';
  }
  const frame = getTopFrame(stack);

  expect(frame!.file).toBe(expectedFile);
});

it('should return the error cause if there is one', () => {
  const error = new Error('Test exception');
  // TODO pass `cause` to the `Error` constructor when lowest supported Node version is 16.9.0 and above
  // See https://github.com/nodejs/node/blob/main/doc/changelogs/CHANGELOG_V16.md#error-cause
  error.cause = new Error('Cause Error');
  const message = formatExecError(
    error,
    {
      rootDir: '',
      testMatch: [],
    },
    {
      noStackTrace: false,
    },
  );
  expect(message).toMatchSnapshot();
});
Repository: facebook/jest
File: packages/jest-message-util/src/index.ts (TypeScript)

Code before:
): string => {
  if (!error || typeof error === 'number') {
    error = new Error(`Expected an Error, but "${String(error)}" was thrown`);
    error.stack = '';
  }

  let message, stack;
  let cause = '';

  if (typeof error === 'string' || !error) {
    error || (error = 'EMPTY ERROR');
    message = '';
    stack = error;
  } else {
    message = error.message;
    stack =
      typeof error.stack === 'string'
        ? error.stack
        : `thrown: ${prettyFormat(error, {maxDepth: 3})}`;
    if ('cause' in error) {
      const prefix = '\n\nCause:\n';
      if (typeof error.cause === 'string') {
        cause += `${prefix}${error.cause}`;
      } else if (typeof error.cause === 'number') {
        cause += `${prefix}${error.cause.toString()}`;
      } else if (error.cause instanceof Error) {
        const formatted = formatExecError(
          error.cause,
          config,
          options,
          testPath,
          reuseMessage,
          true,
        );
        cause += `${prefix}${formatted}`;
      }
    }
  }
  if (cause !== '') {
    cause = indentAllLines(cause);
  }

  const separated = separateMessageFromStack(stack || '');
  stack = separated.stack;

  if (separated.message.includes(trim(message))) {
    // Often stack trace already contains the duplicate of the message
    message = separated.message;
  }

  message = checkForCommonEnvironmentErrors(message);

Review comment (suggestion):
use `types.isNativeError` instead - it works cross realms: https://nodejs.org/api/util.html#utiltypesisnativeerrorvalue

Code after:
  noTitle?: boolean,
): string => {
  if (!error || typeof error === 'number') {
    error = new Error(`Expected an Error, but "${String(error)}" was thrown`);
    error.stack = '';
  }

  let message, stack;
  let cause = '';

  if (typeof error === 'string' || !error) {
    error || (error = 'EMPTY ERROR');
    message = '';
    stack = error;
  } else {
    message = error.message;
    stack =
      typeof error.stack === 'string'
        ? error.stack
        : `thrown: ${prettyFormat(error, {maxDepth: 3})}`;
    if ('cause' in error) {
      const prefix = '\n\nCause:\n';
      if (typeof error.cause === 'string' || typeof error.cause === 'number') {
        cause += `${prefix}${error.cause}`;
      } else if (types.isNativeError(error.cause)) {
        const formatted = formatExecError(
          error.cause,
          config,
          options,
          testPath,
          reuseMessage,
          true,
        );
        cause += `${prefix}${formatted}`;
      }
    }
  }
  if (cause !== '') {
    cause = indentAllLines(cause);
  }

  const separated = separateMessageFromStack(stack || '');
  stack = separated.stack;

  if (separated.message.includes(trim(message))) {
    // Often stack trace already contains the duplicate of the message
    message = separated.message;
  }

  message = checkForCommonEnvironmentErrors(message);