test(xaxis): prevent Date data test from failing on non-English locales#6694
Conversation
WalkthroughA new internal helper function Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I’ve opened a PR that fixes this by normalizing the timezone text in the test helper. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/helper/expectAxisTicks.ts (1)
12-15: Constrain date normalization so non-date tick labels aren’t rewritten
normalizeDateStringcurrently replaces the first parenthetical substring on any non-empty label, so a generic label like"Value (ms)"would become"Value (Coordinated Universal Time)". BecauseexpectXAxisTicks/expectYAxisTicksare shared helpers, this can subtly break or confuse non-date axis tests that rely on parentheses for units or other context.Consider tightening the logic so you only normalize labels that actually look like
Date.toString()output, for example:
- Only normalize when the string contains a GMT/UTC pattern and the parenthetical is at the end of the string, or
- Guard with a more specific regex for the trailing timezone segment instead of
/\(.*?\)/on arbitrary text.Optionally, you could also add an explicit return type to
normalizeDateString(e.g.string | null) to keep TS return types clear, in line with the project’s TS guidelines.Also applies to: 21-22, 32-33
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.json(3 hunks)test/helper/expectAxisTicks.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (DEVELOPING.md)
Run type checking on the codebase using
npm run check-types
**/*.{ts,tsx}: Never useanytype (implicit or explicit) in TypeScript code
Preferunknownoveranyand refine the type in TypeScript
Type function parameters and return values explicitly in TypeScript, do not rely on implicit any or inference; exceptions are React components and trivial functions
Do not useastype assertions in TypeScript; the only exception isas const
Files:
test/helper/expectAxisTicks.ts
**/*.{js,ts,tsx}
📄 CodeRabbit inference engine (CONTRIBUTING.md)
Ensure code lints by running
npm run lintand follows Airbnb's Style Guide
Files:
test/helper/expectAxisTicks.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `vi.useFakeTimers()` in all tests due to Redux autoBatchEnhancer dependency on timers and `requestAnimationFrame`
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Call `vi.runOnlyPendingTimers()` to advance timers after renders when not using `createSelectorTestCase` helper, and avoid `vi.runAllTimers()` to prevent infinite loops
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `userEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })` or the `userEventSetup` helper function from `test/helper/userEventSetup.ts` when creating userEvent instances
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : When testing tooltips on hover, use `vi.runOnlyPendingTimers()` after each `userEvent.hover()` call or use the `showTooltip` helper function from `tooltipTestHelpers.ts` to account for requestAnimationFrame delays
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.699Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Update Storybook stories when APIs have been changed to ensure they work as expected
📚 Learning: 2025-11-25T01:22:59.699Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.699Z
Learning: Applies to test/component/**/*.spec.tsx : Use React Testing Library for testing component interactions and behavior upon rendering
Applied to files:
package.json
📚 Learning: 2025-11-25T01:22:59.699Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.699Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Use Storybook for smoke tests and add play functions with assertions for actual tests
Applied to files:
package.json
📚 Learning: 2025-11-25T01:22:59.699Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.699Z
Learning: Applies to storybook/stories/**/*.stories.tsx : Update Storybook stories when APIs have been changed to ensure they work as expected
Applied to files:
package.json
📚 Learning: 2025-11-25T01:22:48.279Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-11-25T01:22:48.279Z
Learning: Applies to storybook/**/*.stories.{ts,tsx} : Use Storybook stories in the `storybook` directory for high-fidelity component examples that will be published and used for visual regression testing
Applied to files:
package.json
📚 Learning: 2025-11-25T01:22:48.279Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: DEVELOPING.md:0-0
Timestamp: 2025-11-25T01:22:48.279Z
Learning: Applies to src/**/*.{js,jsx,ts,tsx} : Run ESLint and Prettier on the codebase using `npm run lint`
Applied to files:
package.json
📚 Learning: 2025-11-25T01:22:59.699Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: CONTRIBUTING.md:0-0
Timestamp: 2025-11-25T01:22:59.699Z
Learning: Applies to **/*.{js,ts,tsx} : Ensure code lints by running `npm run lint` and follows Airbnb's Style Guide
Applied to files:
package.json
📚 Learning: 2025-11-25T01:23:08.237Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use the `expectLastCalledWith` helper function instead of `expect(spy).toHaveBeenLastCalledWith(...)` for better typing and autocompletion
Applied to files:
test/helper/expectAxisTicks.ts
📚 Learning: 2025-11-25T01:23:08.237Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `userEvent.setup({ advanceTimers: vi.runOnlyPendingTimers })` or the `userEventSetup` helper function from `test/helper/userEventSetup.ts` when creating userEvent instances
Applied to files:
test/helper/expectAxisTicks.ts
📚 Learning: 2025-11-25T01:23:08.237Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Mock `getBoundingClientRect` in tests using the helper function provided in `test/helper/MockGetBoundingClientRect.ts`
Applied to files:
test/helper/expectAxisTicks.ts
📚 Learning: 2025-11-25T01:23:08.237Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Prefer to use the `createSelectorTestCase` helper function when writing or modifying tests
Applied to files:
test/helper/expectAxisTicks.ts
📚 Learning: 2025-11-25T01:23:08.237Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : When testing tooltips on hover, use `vi.runOnlyPendingTimers()` after each `userEvent.hover()` call or use the `showTooltip` helper function from `tooltipTestHelpers.ts` to account for requestAnimationFrame delays
Applied to files:
test/helper/expectAxisTicks.ts
📚 Learning: 2025-11-25T01:23:08.237Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Use `vi.useFakeTimers()` in all tests due to Redux autoBatchEnhancer dependency on timers and `requestAnimationFrame`
Applied to files:
test/helper/expectAxisTicks.ts
📚 Learning: 2025-11-25T01:23:08.237Z
Learnt from: CR
Repo: recharts/recharts PR: 0
File: test/README.md:0-0
Timestamp: 2025-11-25T01:23:08.237Z
Learning: Applies to test/**/*.{test,spec}.{ts,tsx} : Call `vi.runOnlyPendingTimers()` to advance timers after renders when not using `createSelectorTestCase` helper, and avoid `vi.runAllTimers()` to prevent infinite loops
Applied to files:
test/helper/expectAxisTicks.ts
🧬 Code graph analysis (1)
test/helper/expectAxisTicks.ts (1)
test/helper/assertNotNull.ts (1)
assertNotNull(1-5)
🔇 Additional comments (1)
package.json (1)
123-124: Verify test and tooling passes after devDependency bumpsThe updated versions for
@testing-library/*,eslint-plugin-react-perf,jsdom, andvitestare all patch-level and look reasonable, but they do touch the test runner and DOM environment, so subtle behavior changes are possible.Please confirm that at least the following still pass on this branch:
npm run lintnpm run check-typesnpm test(or the relevantvitestscripts defined here)If CI already runs these, just ensure it’s green before merging.
Also applies to: 154-155, 159-160, 175-176
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6694 +/- ##
=======================================
Coverage 94.02% 94.02%
=======================================
Files 499 499
Lines 42544 42544
Branches 4873 4873
=======================================
Hits 40001 40001
Misses 2538 2538
Partials 5 5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
you'll need to npm install because of the changes to the package.json. Could also just undo those |
|
is it done now??? |
|
Nope, please undo the changes in the package.json 😅 |
b54e1a4 to
b1aff31
Compare
|
Done! package.json changes are removed also |
|
Yup! Thanks |
Date.toString() returns different timezone labels depending on OS locale.
Because of this, the Date XAxis test failed on some machines.
I added a normalization in expectXAxisTicks so the comparison always uses the same UTC label.
This keeps the test reliable.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.