Skip to content

🧪 QA: Add test for getStartOf edge case #338

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daggerstuff wants to merge 8 commits into
base: staging
Choose a base branch
from
+12 −2
Open

🧪 QA: Add test for getStartOf edge case #338

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
b23cba4
test: add getStartOf test case
google-labs-jules[bot] Apr 1, 2026
0662652
test: add getStartOf test case
google-labs-jules[bot] Apr 1, 2026
235b001
test: add getStartOf test case
google-labs-jules[bot] Apr 1, 2026
f33c6b8
test: add getStartOf test case
google-labs-jules[bot] Apr 1, 2026
6d7b726
test: add getStartOf test case
google-labs-jules[bot] Apr 1, 2026
b33c68e
test: add getStartOf test case
google-labs-jules[bot] Apr 1, 2026
fabe6db
test: add getStartOf test case
google-labs-jules[bot] Apr 1, 2026
129f1d5
test: add getStartOf test case
google-labs-jules[bot] Apr 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/utils/formatDate.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, it, expect } from 'vitest'
import { formatDuration, isValidDate } from './formatDate'
import { formatDuration, isValidDate, getStartOf } from './formatDate'

describe('formatDuration', () => {
it('formats seconds correctly', () => {
Expand Down Expand Up @@ -60,4 +60,14 @@ describe('isValidDate', () => {
expect(isValidDate('2023-01-01T00:00:00+05:00')).toBe(true)
expect(isValidDate('2023-01-01T00:00:00-08:00')).toBe(true)
})
})
})
describe('getStartOf', () => {
it('returns the start of the day', () => {
const date = new Date('2023-05-15T12:30:45Z');
const start = getStartOf(date, 'day');
expect(start.getHours()).toBe(0);
expect(start.getMinutes()).toBe(0);
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P2: New getStartOf test is incomplete: it only checks time fields and does not verify the returned date/day, allowing wrong-day midnight results to pass.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At src/utils/formatDate.test.ts, line 69:

<comment>New `getStartOf` test is incomplete: it only checks time fields and does not verify the returned date/day, allowing wrong-day midnight results to pass.</comment>

<file context>
@@ -60,4 +60,14 @@ describe('isValidDate', () => {
+    const date = new Date('2023-05-15T12:30:45Z');
+    const start = getStartOf(date, 'day');
+    expect(start.getHours()).toBe(0);
+    expect(start.getMinutes()).toBe(0);
+    expect(start.getSeconds()).toBe(0);
+    expect(start.getMilliseconds()).toBe(0);
</file context>
Fix with Cubic

expect(start.getSeconds()).toBe(0);
expect(start.getMilliseconds()).toBe(0);
Comment on lines +68 to +71
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

This test is timezone-dependent and can be flaky. new Date('...Z') creates a UTC timestamp, but getHours()/getMinutes()/... read local time; if getStartOf normalizes to UTC midnight (or if the runtime timezone isn’t UTC), these assertions may fail. Make the assertion unambiguous by either (a) asserting via getUTCHours()/getUTCMinutes()/... (if getStartOf is intended to be UTC-based), or (b) constructing the input date in local time and/or asserting against an explicit expected timestamp/ISO string that matches the intended semantics (local vs UTC).

Suggested change
expect(start.getHours()).toBe(0);
expect(start.getMinutes()).toBe(0);
expect(start.getSeconds()).toBe(0);
expect(start.getMilliseconds()).toBe(0);
expect(start.getUTCHours()).toBe(0);
expect(start.getUTCMinutes()).toBe(0);
expect(start.getUTCSeconds()).toBe(0);
expect(start.getUTCMilliseconds()).toBe(0);

Copilot uses AI. Check for mistakes.
Comment on lines +66 to +71
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Semicolons were introduced in the new test, but the surrounding file uses a no-semicolons style. For consistency and easier maintenance, align the new lines with the existing formatting conventions in this test file.

Suggested change
const date = new Date('2023-05-15T12:30:45Z');
const start = getStartOf(date, 'day');
expect(start.getHours()).toBe(0);
expect(start.getMinutes()).toBe(0);
expect(start.getSeconds()).toBe(0);
expect(start.getMilliseconds()).toBe(0);
const date = new Date('2023-05-15T12:30:45Z')
const start = getStartOf(date, 'day')
expect(start.getHours()).toBe(0)
expect(start.getMinutes()).toBe(0)
expect(start.getSeconds()).toBe(0)
expect(start.getMilliseconds()).toBe(0)

Copilot uses AI. Check for mistakes.
})
Comment on lines +64 to +72
Copy link
Copy Markdown

@charliecreates charliecreates bot Apr 1, 2026

Choose a reason for hiding this comment

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

This test mixes a UTC input ('...Z') with local-time assertions (getHours()/getMinutes()/etc). If getStartOf is implemented using UTC setters/getters (setUTCHours/getUTCHours), this becomes environment-dependent and can be flaky across time zones/CI settings. Also, asserting only the time fields can miss a bug where the day shifts unexpectedly (still 00:00:00.000, but on the wrong date).

Suggestion

Make the expectation deterministic and explicit about the intended semantics (local vs UTC). A robust pattern is to compute the expected value using the same JS semantics and compare timestamps:

it('returns the start of the day', () => {
  const date = new Date('2023-05-15T12:30:45Z')

  const expected = new Date(date)
  expected.setHours(0, 0, 0, 0) // local start-of-day

  const start = getStartOf(date, 'day')
  expect(start.getTime()).toBe(expected.getTime())
})

If getStartOf is intended to be UTC-based, use setUTCHours(0,0,0,0) and assert with getTime() similarly.

Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this change.

})
Loading