From 6d41c1657e5d5f6ba0648af1e8d93e577a9977e1 Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Wed, 26 Aug 2026 21:04:13 -0400 Subject: [PATCH 1/2] fix: various ts issues --- .../ActivityCalendarHeatmap.test.tsx | 69 ++++++++++++++ .../dashboard/ActivityCalendarHeatmap.tsx | 17 +++- .../dashboard/RecentActivitiesList.test.tsx | 94 +++++++++++++++++++ .../dashboard/RecentActivitiesList.tsx | 12 ++- packages/web/src/hooks/useActivities.ts | 7 ++ .../web/src/hooks/useTrainingMomentum.test.ts | 20 ++-- .../web/src/pages/ActivitiesPage.test.tsx | 5 + packages/web/src/utils/activityStatus.test.ts | 27 ++++++ packages/web/src/utils/activityStatus.ts | 10 +- packages/web/src/utils/colorTokens.ts | 12 ++- 10 files changed, 259 insertions(+), 14 deletions(-) create mode 100644 packages/web/src/components/dashboard/RecentActivitiesList.test.tsx diff --git a/packages/web/src/components/dashboard/ActivityCalendarHeatmap.test.tsx b/packages/web/src/components/dashboard/ActivityCalendarHeatmap.test.tsx index a5437931f..80603f67c 100644 --- a/packages/web/src/components/dashboard/ActivityCalendarHeatmap.test.tsx +++ b/packages/web/src/components/dashboard/ActivityCalendarHeatmap.test.tsx @@ -8,6 +8,7 @@ import { mockDailySportDataReturn, emptyDailySportData, } from "../../test/fixtures/sportConfig"; +import type { MultiSportData } from "../../hooks/useDailySportData"; // Mock useDailySportData hook vi.mock("../../hooks/useDailySportData", () => ({ @@ -697,4 +698,72 @@ describe("ActivityCalendarHeatmap", () => { }); }); }); + + describe("malformed wire data", () => { + // protojson omits proto3 defaults unless EmitUnpopulated is set, so a + // DailyActivity whose `activities` is 0 can arrive with the field absent. + // Before the `?? 0` guard, that `undefined` poisoned the day's count to NaN, + // which propagated through totalActivities into a literal "NaN activities" + // header. SportBucketSchema doesn't descend into DailyActivity, so the dev + // contract-drift warning doesn't fire either. + it("treats a missing `activities` field as zero instead of rendering NaN", () => { + const poisoned = { + cycling: { + "2026-01-02": { + distanceMeters: 45000, + timeMinutes: 90, + elevationMeters: 500, + // `activities` deliberately absent — the wire shape under test. + activityIds: [1], + }, + "2026-01-03": { + distanceMeters: 30000, + timeMinutes: 60, + elevationMeters: 300, + activities: 2, + activityIds: [2], + }, + }, + running: {}, + yoga: {}, + } as unknown as MultiSportData; + + mockUseDailySportData.mockReturnValue(mockDailySportDataReturn({ data: poisoned })); + + const { container } = render(); + + expect(container.textContent).not.toContain("NaN"); + // The one well-formed day still counts; the absent field contributes 0. + expect(screen.getByText(/2 activities in/)).toBeInTheDocument(); + }); + + // Characterisation, not regression: this passes with or without the `?? 0` + // guard, because the cell already floors its input with `|| 0` and + // `NaN || 0` is 0. It pins that invariant so the audit's "paints the + // brightest colour" claim can't become true if someone drops the `|| 0`. + it("does not paint a day with a missing count as the busiest colour", () => { + const poisoned = { + cycling: { + "2026-01-02": { + distanceMeters: 45000, + timeMinutes: 90, + elevationMeters: 500, + activityIds: [1], + }, + }, + running: {}, + yoga: {}, + } as unknown as MultiSportData; + + mockUseDailySportData.mockReturnValue(mockDailySportDataReturn({ data: poisoned })); + + render(); + + // Cell labels are the accessible surface for the count; a poisoned day + // must read as 0, not NaN, and so must take the dim bucket. + const cell = screen.getByRole("img", { name: /^2026-01-02:/ }); + expect(cell).toHaveAttribute("aria-label", "2026-01-02: 0 activities"); + expect(cell).toHaveStyle({ background: "var(--color-slate-light)" }); + }); + }); }); diff --git a/packages/web/src/components/dashboard/ActivityCalendarHeatmap.tsx b/packages/web/src/components/dashboard/ActivityCalendarHeatmap.tsx index ee5979daf..8723e6a47 100644 --- a/packages/web/src/components/dashboard/ActivityCalendarHeatmap.tsx +++ b/packages/web/src/components/dashboard/ActivityCalendarHeatmap.tsx @@ -28,9 +28,16 @@ const INTENSITY_COLORS = [ "var(--color-intensity-4)", // 6+ activities - neon magenta ] as const; -/** Get color for activity count */ +/** + * Get color for activity count. + * + * Defence in depth: the sole caller already floors its input with `|| 0`, so a + * non-finite count cannot reach here today. The guard exists because the + * fallthrough returns the *brightest* bucket — if that `|| 0` were ever removed, + * a NaN would silently paint a day as the busiest of the year rather than fail. + */ function getIntensityColor(count: number): string { - if (count === 0) return INTENSITY_COLORS[0]; + if (!Number.isFinite(count) || count <= 0) return INTENSITY_COLORS[0]; if (count === 1) return INTENSITY_COLORS[1]; if (count <= 3) return INTENSITY_COLORS[2]; if (count <= 5) return INTENSITY_COLORS[3]; @@ -252,7 +259,11 @@ export default function ActivityCalendarHeatmap({ const sportData = data[sport]; if (sportData) { Object.entries(sportData).forEach(([date, activity]) => { - counts[date] = (counts[date] || 0) + activity.activities; + // `?? 0`: protojson omits proto3 defaults unless EmitUnpopulated is set, so a + // DailyActivity with activities: 0 arrives with the field absent. Unguarded, + // `undefined` poisons this date to NaN, which then propagates through + // totalActivities into a "NaN activities" header. + counts[date] = (counts[date] || 0) + (activity.activities ?? 0); }); } }); diff --git a/packages/web/src/components/dashboard/RecentActivitiesList.test.tsx b/packages/web/src/components/dashboard/RecentActivitiesList.test.tsx new file mode 100644 index 000000000..345bc7cd8 --- /dev/null +++ b/packages/web/src/components/dashboard/RecentActivitiesList.test.tsx @@ -0,0 +1,94 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { render, screen, fireEvent } from "@testing-library/react"; +import RecentActivitiesList from "./RecentActivitiesList"; +import type { UseActivitiesResult } from "../../hooks/useActivities"; +import type { ActivitySummary } from "../../api/activities"; + +vi.mock("@tanstack/react-router", () => ({ useNavigate: () => vi.fn() })); +vi.mock("../../hooks/useAuth", () => ({ useAuth: () => ({ user: { uid: "u1" } }) })); +vi.mock("../../hooks/useDashboardGoalData", () => ({ + useDashboardGoalData: () => ({ sportData: [], distanceUnit: "kilometers" }), +})); +vi.mock("../../contexts/ThemeContext", () => ({ useTheme: () => ({ resolvedTheme: "dark" }) })); +vi.mock("../../hooks/useActivities", () => ({ useActivities: vi.fn() })); + +import { useActivities } from "../../hooks/useActivities"; +const mockUseActivities = vi.mocked(useActivities); + +function activity(id: number): ActivitySummary { + return { + id: String(id), + name: `Activity ${id}`, + type: "Ride", + sport: "cycling", + startDateLocal: "2026-01-0" + ((id % 9) + 1) + "T08:00:00", + distanceMeters: 1000 * id, + movingTimeSeconds: 600, + elevationMeters: 10, + hasRoute: false, + }; +} + +describe("RecentActivitiesList pagination", () => { + beforeEach(() => vi.clearAllMocks()); + + // Regression: at the last *local* page with hasMore, handleNextPage calls the + // async loadMore() and optimistically does setPage(p + 1). On the re-render that + // setPage triggers, the fetch has not resolved, so activities.length — and thus + // totalPages — is unchanged, and the clamp + // clampedPage = Math.min(page, totalPages - 1) + // snapped page straight back. The user's first "Older" click at each server-page + // boundary was a no-op and a second click was required. It only reproduced when + // the next page wasn't already cached, i.e. the common case, which is why it + // survived. The clamp is now skipped while isLoadingMore is true. + it("advances on the first Older click at a server-page boundary", () => { + const firstPage = [activity(1), activity(2)]; + let isLoadingMore = false; + + mockUseActivities.mockImplementation((): UseActivitiesResult => ({ + activities: firstPage, + isLoading: false, + error: null, + hasMore: true, + isLoadingMore, + // Mirrors fetchNextPage: the flag flips now, the data lands later. + loadMore: () => { + isLoadingMore = true; + }, + retry: vi.fn(), + })); + + render(); + + // pageSize 2 with 2 activities => one local page, so page 0 is the boundary. + expect(screen.getByText("1/+")).toBeInTheDocument(); + + fireEvent.click(screen.getByLabelText("Older activities")); + + // Before the fix this read "1/+" again — the click was swallowed. + expect(screen.getByText("2/+")).toBeInTheDocument(); + }); + + it("still clamps an out-of-range page when nothing is being fetched", () => { + // The clamp's original purpose: a resize shrinks pageSize-derived totalPages + // and the current page falls out of range. With no fetch in flight it must + // still pull the page back rather than render an empty slice. + mockUseActivities.mockImplementation((): UseActivitiesResult => ({ + activities: [activity(1)], + isLoading: false, + error: null, + hasMore: true, + isLoadingMore: false, + loadMore: vi.fn(), + retry: vi.fn(), + })); + + const { rerender } = render(); + fireEvent.click(screen.getByLabelText("Older activities")); + + // hasMore is true but loadMore never resolves and isLoadingMore stays false, + // so the advance is not protected and the clamp reels it back in. + rerender(); + expect(screen.getByText("1/+")).toBeInTheDocument(); + }); +}); diff --git a/packages/web/src/components/dashboard/RecentActivitiesList.tsx b/packages/web/src/components/dashboard/RecentActivitiesList.tsx index a3618c080..80ab15c7a 100644 --- a/packages/web/src/components/dashboard/RecentActivitiesList.tsx +++ b/packages/web/src/components/dashboard/RecentActivitiesList.tsx @@ -174,12 +174,18 @@ export default function RecentActivitiesList({ // Memoized: useActivities' demo path filters on the filter object's identity, // so an inline literal would re-filter (and re-render) every render signed out. const filter = useMemo(() => ({ from, to, sports: [], limit: 20 }), [from, to]); - const { activities, isLoading, error, hasMore, loadMore } = useActivities(filter); + const { activities, isLoading, error, hasMore, isLoadingMore, loadMore } = useActivities(filter); const totalPages = Math.ceil(activities.length / pageSize); - // Clamp page if pageSize changed (e.g. container resized) and current page is now out of range + // Clamp page if pageSize changed (e.g. container resized) and current page is now out of range. + // + // Skip the clamp while a next-page fetch is in flight. handleNextPage advances + // past the last *local* page and lets loadMore() fill it in, but activities.length + // has not grown yet on that render — so clamping here would snap the page back + // before the data lands and silently eat the user's click. It only reproduces + // when the next server page isn't already cached, i.e. the common case. const clampedPage = Math.min(page, Math.max(0, totalPages - 1)); - if (clampedPage !== page) setPage(clampedPage); + if (clampedPage !== page && !isLoadingMore) setPage(clampedPage); const startIdx = clampedPage * pageSize; const visibleActivities = activities.slice(startIdx, startIdx + pageSize); diff --git a/packages/web/src/hooks/useActivities.ts b/packages/web/src/hooks/useActivities.ts index 1653592c7..309f97562 100644 --- a/packages/web/src/hooks/useActivities.ts +++ b/packages/web/src/hooks/useActivities.ts @@ -64,6 +64,12 @@ export interface UseActivitiesResult { isLoading: boolean; error: Error | null; hasMore: boolean; + /** + * A next-page fetch is in flight. Distinct from `isLoading`, which covers only + * the initial load — consumers that optimistically advance a page on loadMore() + * need to know the difference so they don't undo the advance mid-fetch. + */ + isLoadingMore: boolean; loadMore: () => void; retry: () => void; } @@ -123,6 +129,7 @@ export function useActivities(filter: Omit): UseAc isLoading: authLoading || (!!user && isFetching && !isFetchingNextPage && !data), error: error, hasMore: !user ? false : !!hasNextPage, + isLoadingMore: isFetchingNextPage, loadMore: () => { void fetchNextPage(); }, diff --git a/packages/web/src/hooks/useTrainingMomentum.test.ts b/packages/web/src/hooks/useTrainingMomentum.test.ts index 7e3269234..fa61acf07 100644 --- a/packages/web/src/hooks/useTrainingMomentum.test.ts +++ b/packages/web/src/hooks/useTrainingMomentum.test.ts @@ -2,6 +2,14 @@ import { describe, it, expect } from "vitest"; import { renderHook } from "@testing-library/react"; import { useTrainingMomentum } from "./useTrainingMomentum"; import type { DistanceEntry } from "../types/activity"; +// toLocalDateString, not toISOString(): these fixtures mean "N *local* calendar +// days ago", but toISOString() formats the UTC date. In a UTC-negative timezone +// an evening test run pushed the string to the next UTC day, so a fixture built +// as "8 days ago" was really 7 local days back. That made the staleness +// assertions below pass only because isActivityDataStale had a matching +// UTC-vs-local skew; with the clock anchored correctly the mismatch surfaced as +// a time-of-day-dependent failure. Keep both sides on the local convention. +import { toLocalDateString } from "../utils/dateUtils"; describe("useTrainingMomentum", () => { const createDistanceData = (entries: Array<{ x: string; y: number }>): DistanceEntry[] => { @@ -15,7 +23,7 @@ describe("useTrainingMomentum", () => { for (let i = 9; i >= 0; i--) { const date = new Date(today); date.setDate(date.getDate() - i); - data.push({ x: date.toISOString().split("T")[0]!, y: 100 + (9 - i) ** 2 }); + data.push({ x: toLocalDateString(date), y: 100 + (9 - i) ** 2 }); } const { result } = renderHook(() => useTrainingMomentum(data, 10)); @@ -35,7 +43,7 @@ describe("useTrainingMomentum", () => { for (let i = 9; i >= 0; i--) { const date = new Date(today); date.setDate(date.getDate() - i); - data.push({ x: date.toISOString().split("T")[0]!, y: distance }); + data.push({ x: toLocalDateString(date), y: distance }); distance += increments[9 - i]!; } @@ -51,14 +59,14 @@ describe("useTrainingMomentum", () => { for (let i = 15; i >= 8; i--) { const date = new Date(); date.setDate(date.getDate() - i); - data.push({ x: date.toISOString().split("T")[0]!, y: 100 + (15 - i) * 10 }); + data.push({ x: toLocalDateString(date), y: 100 + (15 - i) * 10 }); } // Add extended data (flat-line) for the last 8 days const lastDistance = data.at(-1)!.y; for (let i = 7; i >= 0; i--) { const date = new Date(); date.setDate(date.getDate() - i); - data.push({ x: date.toISOString().split("T")[0]!, y: lastDistance }); + data.push({ x: toLocalDateString(date), y: lastDistance }); } const { result } = renderHook(() => useTrainingMomentum(data, 10)); @@ -74,7 +82,7 @@ describe("useTrainingMomentum", () => { for (let i = 7; i >= 0; i--) { const date = new Date(today); date.setDate(date.getDate() - i); - data.push({ x: date.toISOString().split("T")[0]!, y: 100 + (7 - i) * 10 }); + data.push({ x: toLocalDateString(date), y: 100 + (7 - i) * 10 }); } const { result } = renderHook(() => useTrainingMomentum(data, 10)); @@ -92,7 +100,7 @@ describe("useTrainingMomentum", () => { dates.forEach((daysAgo, idx) => { const date = new Date(today); date.setDate(date.getDate() - daysAgo); - data.push({ x: date.toISOString().split("T")[0]!, y: values[idx]! }); + data.push({ x: toLocalDateString(date), y: values[idx]! }); }); const { result } = renderHook(() => useTrainingMomentum(data, 10)); diff --git a/packages/web/src/pages/ActivitiesPage.test.tsx b/packages/web/src/pages/ActivitiesPage.test.tsx index fae05b53f..57031b98a 100644 --- a/packages/web/src/pages/ActivitiesPage.test.tsx +++ b/packages/web/src/pages/ActivitiesPage.test.tsx @@ -81,6 +81,7 @@ describe("ActivitiesPage", () => { isLoading: false, error: null, hasMore: false, + isLoadingMore: false, loadMore: vi.fn(), retry: vi.fn(), }); @@ -98,6 +99,7 @@ describe("ActivitiesPage", () => { isLoading: false, error: null, hasMore: false, + isLoadingMore: false, loadMore: vi.fn(), retry: vi.fn(), }); @@ -116,6 +118,7 @@ describe("ActivitiesPage", () => { isLoading: false, error: null, hasMore: false, + isLoadingMore: false, loadMore: vi.fn(), retry: vi.fn(), }); @@ -238,6 +241,7 @@ describe("ActivitiesPage", () => { isLoading: true, error: null, hasMore: false, + isLoadingMore: false, loadMore: vi.fn(), retry: vi.fn(), }); @@ -256,6 +260,7 @@ describe("ActivitiesPage", () => { isLoading: false, error: null, hasMore: false, + isLoadingMore: false, loadMore: vi.fn(), retry: vi.fn(), }); diff --git a/packages/web/src/utils/activityStatus.test.ts b/packages/web/src/utils/activityStatus.test.ts index 1e756d5fb..5e89751ab 100644 --- a/packages/web/src/utils/activityStatus.test.ts +++ b/packages/web/src/utils/activityStatus.test.ts @@ -110,6 +110,33 @@ describe("isActivityDataStale", () => { expect(isActivityDataStale([])).toBe(true); }); + // Regression: "days since" used to be computed as + // daysBetween(new Date(lastActivityString), new Date()) + // which subtracts a UTC-midnight-parsed date from a *live instant* and floors. + // The fractional time-of-day therefore leaked into the day count, so the same + // data gave different answers depending on when the page was loaded. On the + // boundary day in America/New_York (UTC-4), an evening load crossed into the + // next UTC day and reported 8 days instead of 7 — flipping the MomentumIndicator + // stale badge a full day early. Anchoring "today" with getTodayUtcAnchored() + // puts both operands on the same convention. + describe("is independent of the time of day the page is loaded", () => { + // Exactly 7 local calendar days before 2025-10-22, i.e. on the boundary + // (STALE_ACTIVITY_DAYS = 7, and the check is `>` not `>=`). + const data: DistanceEntry[] = [ + { x: "2025-10-01", y: 0 }, + { x: "2025-10-15", y: 100 }, + ]; + + it.each([ + ["morning", "2025-10-22T13:00:00Z"], // 09:00 EDT + ["midday", "2025-10-22T16:00:00Z"], // 12:00 EDT + ["evening", "2025-10-23T01:00:00Z"], // 21:00 EDT, still Oct 22 locally + ])("reports not-stale on the boundary day (%s load)", (_label, instant) => { + vi.setSystemTime(new Date(instant)); + expect(isActivityDataStale(data)).toBe(false); + }); + }); + it("returns true when no activities found", () => { const data: DistanceEntry[] = [ { x: "2025-10-01", y: 100 }, diff --git a/packages/web/src/utils/activityStatus.ts b/packages/web/src/utils/activityStatus.ts index 360b1bea4..1e439fc5b 100644 --- a/packages/web/src/utils/activityStatus.ts +++ b/packages/web/src/utils/activityStatus.ts @@ -8,6 +8,7 @@ import type { DistanceEntry } from "../types/activity"; import { TRAINING_CONSTANTS } from "../constants/training"; import { daysBetween } from "./dateCalculations"; +import { getTodayUtcAnchored } from "./dateUtils"; /** * Finds the date of the last actual activity @@ -76,7 +77,14 @@ export function isActivityDataStale( const lastActivityDate = findLastActivityDate(distanceData); if (!lastActivityDate) return true; - const today = new Date(); + // getTodayUtcAnchored(), not new Date(): findLastActivityDate parses a + // "YYYY-MM-DD" chart string, which JS reads as UTC midnight (dateUtils' + // "Convention B"). A raw `new Date()` is a live instant, so daysBetween would + // subtract a UTC midnight from a local wall-clock time and floor the result — + // making "days since" depend on the time of day the page was loaded. In + // America/New_York an evening load reported one day more than the true local + // calendar gap, flipping the stale badge a day early. + const today = getTodayUtcAnchored(); const daysSinceActivity = daysBetween(lastActivityDate, today); return daysSinceActivity > staleThresholdDays; diff --git a/packages/web/src/utils/colorTokens.ts b/packages/web/src/utils/colorTokens.ts index 77b8458e5..bef34af8c 100644 --- a/packages/web/src/utils/colorTokens.ts +++ b/packages/web/src/utils/colorTokens.ts @@ -72,6 +72,12 @@ export interface Rgb { * `#rgb`, `#rrggbb`, and `rgb()`/`rgba()`. Returns null for anything else, so * callers can fall back rather than render a broken color. */ +/** Clamp a parsed colour channel into the valid 0-255 range. */ +function clampChannel(n: number): number { + if (!Number.isFinite(n)) return 0; + return Math.min(255, Math.max(0, Math.round(n))); +} + export function parseRgb(color: string): Rgb | null { // Trim once, up front: `getComputedStyle().getPropertyValue()` commonly returns a // leading space, and anchoring the rgb() pattern against an untrimmed string made it @@ -92,7 +98,11 @@ export function parseRgb(color: string): Rgb | null { } const fn = value.match(/^rgba?\(\s*([\d.]+)[\s,]+([\d.]+)[\s,]+([\d.]+)/i); if (fn) { - return { r: Math.round(+fn[1]!), g: Math.round(+fn[2]!), b: Math.round(+fn[3]!) }; + // Clamp: the hex branch is bounded by its 0xff masks, but this one accepts any + // digits the regex matches, so a malformed `rgb(300, 0, 0)` would escape as + // r = 300 and skew every downstream luminance/contrast computation instead of + // being rejected or corrected at the boundary. + return { r: clampChannel(+fn[1]!), g: clampChannel(+fn[2]!), b: clampChannel(+fn[3]!) }; } return null; } From 1c0e4fe06ef16fb9edeaf9608ce0470b4787da63 Mon Sep 17 00:00:00 2001 From: Andy Eschbacher Date: Thu, 27 Aug 2026 05:39:36 -0400 Subject: [PATCH 2/2] fix: tz bug for misconfigured env --- packages/web/src/utils/activityStatus.test.ts | 30 +++++++++++----- packages/web/vite.config.ts | 34 ++++++++++++++++--- 2 files changed, 50 insertions(+), 14 deletions(-) diff --git a/packages/web/src/utils/activityStatus.test.ts b/packages/web/src/utils/activityStatus.test.ts index 5e89751ab..b263009e4 100644 --- a/packages/web/src/utils/activityStatus.test.ts +++ b/packages/web/src/utils/activityStatus.test.ts @@ -97,9 +97,15 @@ describe("findLastActivityDate", () => { describe("isActivityDataStale", () => { beforeEach(() => { - // Mock current date to October 22, 2025 + // Mock current date to October 22, 2025. + // Local components, not a "Z" instant: isActivityDataStale anchors "today" + // via getTodayUtcAnchored(), which reads the *local* calendar date. A fixed + // UTC instant lands on a different local day in far-positive zones (12:00Z + // is already Oct 23 in Pacific/Auckland), which would shift every "N days + // ago" assertion below by one. vite.config.ts only defaults TZ for tests, so + // an explicit override is allowed and this has to hold in any zone. vi.useFakeTimers(); - vi.setSystemTime(new Date("2025-10-22T12:00:00Z")); + vi.setSystemTime(new Date(2025, 9, 22, 12, 0, 0)); }); afterEach(() => { @@ -115,8 +121,8 @@ describe("isActivityDataStale", () => { // which subtracts a UTC-midnight-parsed date from a *live instant* and floors. // The fractional time-of-day therefore leaked into the day count, so the same // data gave different answers depending on when the page was loaded. On the - // boundary day in America/New_York (UTC-4), an evening load crossed into the - // next UTC day and reported 8 days instead of 7 — flipping the MomentumIndicator + // boundary day in a UTC-negative zone, an evening load crossed into the next + // UTC day and reported 8 days instead of 7 — flipping the MomentumIndicator // stale badge a full day early. Anchoring "today" with getTodayUtcAnchored() // puts both operands on the same convention. describe("is independent of the time of day the page is loaded", () => { @@ -127,12 +133,18 @@ describe("isActivityDataStale", () => { { x: "2025-10-15", y: 100 }, ]; + // Built from LOCAL components on purpose. vite.config.ts only *defaults* + // TZ to America/New_York for test runs (`??=`, "Respects an explicit TZ + // override"), so hardcoded `Z` instants would silently mean a different + // local calendar day under `TZ=UTC` or a UTC-positive zone and this + // assertion would flip. `new Date(y, m, d, h)` is local in every zone, so + // all three cases stay on local 2025-10-22 wherever the suite runs. it.each([ - ["morning", "2025-10-22T13:00:00Z"], // 09:00 EDT - ["midday", "2025-10-22T16:00:00Z"], // 12:00 EDT - ["evening", "2025-10-23T01:00:00Z"], // 21:00 EDT, still Oct 22 locally - ])("reports not-stale on the boundary day (%s load)", (_label, instant) => { - vi.setSystemTime(new Date(instant)); + ["morning", 9], + ["midday", 12], + ["evening", 21], + ])("reports not-stale on the boundary day (%s load)", (_label, hour) => { + vi.setSystemTime(new Date(2025, 9, 22, hour, 0, 0)); expect(isActivityDataStale(data)).toBe(false); }); }); diff --git a/packages/web/vite.config.ts b/packages/web/vite.config.ts index 315f87f7e..782eeed60 100644 --- a/packages/web/vite.config.ts +++ b/packages/web/vite.config.ts @@ -5,12 +5,36 @@ import tailwindcss from "@tailwindcss/vite"; import { execSync } from "child_process"; import { fileURLToPath, URL } from "node:url"; -// In test runs, pin the timezone to a UTC-negative zone (the sole athlete's) so -// timezone off-by-one bugs surface in CI too — CI otherwise runs UTC, where the -// local-vs-UTC-midnight distinction collapses and the bug hides. Respects an explicit -// TZ override. `pool: "forks"` workers inherit this env, set here before they spawn. +// In test runs, force the timezone to a zone with a non-zero UTC offset (default: +// the sole athlete's) so timezone off-by-one bugs surface in CI too — under UTC the +// local-vs-UTC-midnight distinction collapses and this whole class of bug hides. +// `pool: "forks"` workers inherit this env, set here before they spawn. +// +// A deliberate override to another real zone is respected: running the suite under +// Asia/Tokyo or Europe/Berlin is a useful check and still exercises the offset. +// A UTC-equivalent value is NOT respected, because it silently disables the guard +// rather than changing what it tests. Verified by mutation: reverting +// getTodayLocalMidnight() to the UTC-anchored form that +// `harden-getcurrentlocaldate-against-utc-anchored-local-time-mixup` fixed is caught +// under New_York, Tokyo and Berlin — and passes 46/46 under TZ=UTC. +// +// This was previously `??=`, which had two holes: TZ=UTC was honoured outright, and +// TZ="" is neither null nor undefined so `??=` never fired and Node resolved +// Etc/Unknown (effectively UTC). Either one turned the guard off with no signal. if (process.env.VITEST) { - process.env.TZ ??= "America/New_York"; + const requested = process.env.TZ?.trim(); + const isUtcEquivalent = + !requested || /^(UTC|GMT|Z|Zulu|Universal|Etc\/(UTC|GMT|Zulu|Universal|Unknown|GMT[+-]?0))$/i.test(requested); + if (isUtcEquivalent) { + if (requested) { + console.warn( + `[vitest] Ignoring TZ=${JSON.stringify(process.env.TZ)}: a UTC-equivalent zone disables ` + + "the local-vs-UTC regression guard. Forcing America/New_York. " + + "Set TZ to a real offset zone (e.g. Asia/Tokyo) to test another timezone." + ); + } + process.env.TZ = "America/New_York"; + } } // Get git commit hash for versioning