Skip to content

Conversation

@waleedlatif1
Copy link
Collaborator

Summary

  • Replace getMinutes(), getSeconds(), getDate(), etc. with UTC equivalents in schedule tests
  • Fixes test failures in timezones with non-integer hour offsets (e.g., UTC+05:30, UTC+09:30)

Fixes #3045

Type of Change

  • Bug fix

Testing

Tested with multiple timezones: UTC, Asia/Kolkata (UTC+05:30), Australia/Adelaide (UTC+09:30), Asia/Kathmandu (UTC+05:45)

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link

vercel bot commented Jan 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Review Updated (UTC)
docs Skipped Skipped Jan 28, 2026 9:42pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 28, 2026

Greptile Overview

Greptile Summary

This PR fixes timezone-dependent test failures by replacing local date methods with UTC equivalents in schedule test assertions.

Changes Made:

  • Replaced getMinutes() with getUTCMinutes() in interval alignment checks
  • Replaced getSeconds() with getUTCSeconds() in time precision assertions
  • Replaced getFullYear(), getMonth(), and getDate() with UTC equivalents in date validation tests

Why This Matters:
The Croner library returns Date objects with UTC timestamps. When tests used local date methods like getMinutes(), they would return values in the machine's local timezone. In timezones with non-integer hour offsets (e.g., UTC+05:30, UTC+09:30), this caused minute values to be offset, breaking assertions that expected specific minute alignments (e.g., divisibility by 5 or 10).

By using UTC methods, the tests now correctly validate the actual UTC time values that Croner calculates, making the tests timezone-independent.

Confidence Score: 5/5

  • This PR is safe to merge with no risk
  • The changes are purely test-related and fix a real bug. All modifications convert local date methods to UTC equivalents in test assertions, which is the correct approach since Croner returns UTC timestamps. The changes are minimal, focused, and well-tested according to the PR description.
  • No files require special attention

Important Files Changed

Filename Overview
apps/sim/app/api/schedules/[id]/route.test.ts Correctly converts getMinutes() and getSeconds() to UTC equivalents in schedule test assertions
apps/sim/lib/workflows/schedules/utils.test.ts Correctly converts getFullYear(), getMonth(), getDate(), and getMinutes() to UTC equivalents in schedule utility tests

Sequence Diagram

sequenceDiagram
    participant Test as Test Suite
    participant API as Schedule API/Utils
    participant Croner as Croner Library
    participant Date as Date Object
    
    Test->>API: Calculate next run time with cron
    API->>Croner: Generate next occurrence
    Croner->>Croner: Calculate in specified timezone
    Croner-->>API: Return Date object (UTC timestamp)
    API-->>Test: Return Date object
    
    Note over Test,Date: OLD: Test used local methods
    Test->>Date: getMinutes() / getDate() / etc.
    Date-->>Test: Returns LOCAL timezone value ❌
    Note over Test: Fails in timezones like UTC+05:30
    
    Note over Test,Date: NEW: Test uses UTC methods
    Test->>Date: getUTCMinutes() / getUTCDate() / etc.
    Date-->>Test: Returns UTC value ✅
    Note over Test: Works in all timezones
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit c00f05c into staging Jan 28, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/tests branch January 28, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants