test: fix timestamp unit test when not running at UTC #37195
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughA single test in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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 |
ggazzo
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/message-parser/tests/timestamp.test.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
| ], | ||
| [ | ||
| '<t:10:00+00:00>', | ||
| `<t:10:00${new Date().toString().match(/GMT([-+]\d{4})/)?.[1].replace(/(.{3})/, '$1:')}>`, |
There was a problem hiding this comment.
Fragile regex approach may fail in non-standard environments.
The regex /GMT([-+]\d{4})/ assumes a specific format from Date.toString(), which is not standardized and may vary across JavaScript engines, Node versions, or locales. If the regex fails to match, the template string will contain undefined, causing test failures.
Apply this diff to use the more reliable getTimezoneOffset() method:
- `<t:10:00${new Date().toString().match(/GMT([-+]\d{4})/)?.[1].replace(/(.{3})/, '$1:')}>`,
+ (() => {
+ const offset = -new Date().getTimezoneOffset();
+ const hours = Math.floor(Math.abs(offset) / 60).toString().padStart(2, '0');
+ const minutes = (Math.abs(offset) % 60).toString().padStart(2, '0');
+ const sign = offset >= 0 ? '+' : '-';
+ return `<t:10:00${sign}${hours}:${minutes}>`;
+ })(),This approach is explicit, standardized, and works reliably across all JavaScript environments.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| `<t:10:00${new Date().toString().match(/GMT([-+]\d{4})/)?.[1].replace(/(.{3})/, '$1:')}>`, | |
| // … | |
| (() => { | |
| const offset = -new Date().getTimezoneOffset(); | |
| const hours = Math.floor(Math.abs(offset) / 60) | |
| .toString() | |
| .padStart(2, '0'); | |
| const minutes = (Math.abs(offset) % 60) | |
| .toString() | |
| .padStart(2, '0'); | |
| const sign = offset >= 0 ? '+' : '-'; | |
| return `<t:10:00${sign}${hours}:${minutes}>`; | |
| })(), | |
| // … |
🤖 Prompt for AI Agents
In packages/message-parser/tests/timestamp.test.ts around line 106, the code
uses Date.toString() with the fragile regex /GMT([-+]\d{4})/ which can fail in
non-standard environments; replace this with a deterministic calculation using
Date.prototype.getTimezoneOffset(): compute offsetMinutes =
-date.getTimezoneOffset() (note the sign inversion), derive sign ('+' or '-')
and absolute hours/minutes with zero-padding to produce a string in "+HH:MM"
format, and inject that into the template string instead of relying on the regex
match returning undefined.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37195 +/- ##
===========================================
+ Coverage 67.38% 67.39% +0.01%
===========================================
Files 3287 3287
Lines 111684 111684
Branches 20409 20409
===========================================
+ Hits 75258 75270 +12
+ Misses 33738 33731 -7
+ Partials 2688 2683 -5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
rodrigok
force-pushed
the
fix/timestamp-unit-test
branch
from
b789b4c to
64f9bb3
Compare
2 participants
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit