test(science): make definition lookups actually assert

[hiSandog]

Context

The non-seeded tests in test/modules/science.spec.ts were meant to verify that the values returned by faker.science.chemicalElement() and faker.science.unit() exist in the corresponding definitions.

However, each lookup was wrapped in an arrow function passed to expect(...).toBeTruthy():

expect(() => {
  faker.definitions.science.chemical_element.find(
    (element) => element.name === name
  );
}).toBeTruthy();

Since a function is always truthy, expect(...).toBeTruthy() always passes regardless of whether .find() returns a match or undefined. These assertions were effectively no-ops and could not catch a regression where a returned value was no longer present in the definitions.

Change

Call .find() directly so expect(...).toBeTruthy() checks the real lookup result instead of a function reference:

expect(
  faker.definitions.science.chemical_element.find(
    (element) => element.name === name
  )
).toBeTruthy();

Applied to all five affected assertions (element name/symbol/atomic number, unit name/symbol). No production code is touched.

The assertions still pass because chemicalElement() and unit() draw their values from the same definitions via arrayElement, so each lookup now genuinely succeeds.

Validation

• pnpm exec vitest run test/modules/science.spec.ts — 44 tests passed, no type errors
• pnpm exec eslint test/modules/science.spec.ts — no errors
• pnpm exec prettier --write test/modules/science.spec.ts — file unchanged (already formatted)

[netlify]

✅ Deploy Preview for fakerjs ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	4485c8f
🔍 Latest deploy log	https://app.netlify.com/projects/fakerjs/deploys/6a3e32262f8ec80008dc969a
😎 Deploy Preview	https://deploy-preview-3919.fakerjs.dev
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.79%. Comparing base (b517ae9) to head (4485c8f).

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #3919      +/-   ##
==========================================
- Coverage   98.85%   98.79%   -0.07%     
==========================================
  Files         923      923              
  Lines        3224     3224              
  Branches      574      591      +17     
==========================================
- Hits         3187     3185       -2     
- Misses         33       35       +2     
  Partials        4        4              

see 1 file with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Shinigami92]

damn, good catch 😲
we might want to inform vitest team or vitest-lint team to introduce a catching lint rule for such a case

[Copilot]

Pull request overview

Fixes ineffective assertions in the science module’s non-seeded tests by ensuring definition lookups actually evaluate the .find() result (instead of a truthy function reference), so regressions where returned values no longer exist in faker.definitions would now be caught.

Changes:

• Replaced expect(() => { ...find... }).toBeTruthy() with expect(...find...).toBeTruthy() for chemical element name, symbol, and atomic number.
• Applied the same fix for unit name and unit symbol lookups.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[xDivisionByZerox]

damn, good catch 😲 we might want to inform vitest team or vitest-lint team to introduce a catching lint rule for such a case

This is definitely an error on our side. Checking if a function ref is truthy is a valid use case. Think of the following:

// in *.ts file
type MathStrategy = (a: number, b: number) => number;

function getMathStrategy(strategy: string): MathStrategy | undefined {
  switch(strategy) {
    case 'add': return (a, b) => a + b;
    case 'subtract': return (a, b) => a - b;
    case 'multiply': return (a, b) => a * b;
    case 'divide': return (a, b) => a / b;
    default: return undefined;
  }
}

// in *.spec.ts file
it('should return a valid strategy for "add"', () => {
  const strategy = getMathStrategy('add');
  // strategy is nothing else than a anonymous function here
  // doesn't matter if it was previously assigned or not
  expect(strategy).toBeTruthy();
});

Just to be clear, using a strategy/factory pattern that does not return a valid or at least no-op strategy in all cases does not make sense in my mind, but it was the easiest way to write a POC.