Skip to content
Closed
Changes from all commits
Commits
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
107 changes: 107 additions & 0 deletions src/lib/__tests__/yearInReviewUtils.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { describe, it, expect } from "vitest";
import {
buildHourlyHeatmapFromCommitDates,
getMostActiveHour,
getMostActiveDayFromCalendar
} from "../yearInReviewUtils";

describe("buildHourlyHeatmapFromCommitDates", () => {
it("returns a 7x24 heatmap of zeros for an empty array", () => {
const heatmap = buildHourlyHeatmapFromCommitDates([]);
expect(heatmap.length).toBe(7);
heatmap.forEach(day => {
expect(day.length).toBe(24);
day.forEach(hour => expect(hour).toBe(0));
});
Comment on lines +11 to +15

Choose a reason for hiding this comment

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

medium

The current way of checking if the heatmap is all zeros is a bit verbose. You can make this assertion more concise and declarative by comparing it to an expected zero-filled 2D array using toEqual. This also implicitly checks the dimensions of the inner arrays, making the test more robust.

Suggested change
expect(heatmap.length).toBe(7);
heatmap.forEach(day => {
expect(day.length).toBe(24);
day.forEach(hour => expect(hour).toBe(0));
});
expect(heatmap).toEqual(Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0)));

Comment on lines +12 to +15
Copy link

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

forEachのコールバックが値を返しています。

静的解析ツール(Biome)が指摘している通り、forEachのコールバックは値を返すべきではありません。expect(hour).toBe(0)の戻り値が暗黙的に返されています。テストの動作には影響しませんが、コードの意図を明確にするために修正することを推奨します。

♻️ 修正案
     it("returns a 7x24 heatmap of zeros for an empty array", () => {
         const heatmap = buildHourlyHeatmapFromCommitDates([]);
         expect(heatmap.length).toBe(7);
         heatmap.forEach(day => {
             expect(day.length).toBe(24);
-            day.forEach(hour => expect(hour).toBe(0));
+            day.forEach(hour => {
+                expect(hour).toBe(0);
+            });
         });
     });

     it("ignores invalid date strings", () => {
         const heatmap = buildHourlyHeatmapFromCommitDates(["invalid-date", "not-a-date"]);
         heatmap.forEach(day => {
-            day.forEach(hour => expect(hour).toBe(0));
+            day.forEach(hour => {
+                expect(hour).toBe(0);
+            });
         });
     });

Also applies to: 20-22

🧰 Tools
🪛 Biome (2.4.6)

[error] 14-14: This callback passed to forEach() iterable method should not return a value.

(lint/suspicious/useIterableCallbackReturn)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/lib/__tests__/yearInReviewUtils.test.ts` around lines 12 - 15, The test
uses arrow callbacks with implicit returns inside heatmap.forEach which causes
the expect(...) expressions to be implicitly returned; change the callbacks to
use block bodies without returning values (e.g., replace heatmap.forEach(day =>
{ ... }) and the inner day.forEach(hour => { expect(hour).toBe(0); }); or
convert the loops to for...of loops) so that the forEach callbacks do not return
anything; update both occurrences (lines around heatmap.forEach and the inner
day.forEach).

});

it("ignores invalid date strings", () => {
const heatmap = buildHourlyHeatmapFromCommitDates(["invalid-date", "not-a-date"]);
heatmap.forEach(day => {
day.forEach(hour => expect(hour).toBe(0));
});
Comment on lines +20 to +22

Choose a reason for hiding this comment

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

medium

This check for an all-zero heatmap can be more concise. Also, it currently doesn't verify the dimensions of the heatmap array. Using toEqual with an expected zero-filled array would make the test more robust by checking dimensions and values in a single, declarative assertion.

        expect(heatmap).toEqual(Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0)));

});

it("correctly counts commit dates into the right day and hour", () => {
// "2023-10-01T12:00:00Z" -> Sunday (0), 12th hour
// "2023-10-02T14:30:00Z" -> Monday (1), 14th hour
// "2023-10-02T14:45:00Z" -> Monday (1), 14th hour
// "2023-10-07T23:59:59Z" -> Saturday (6), 23rd hour
const commitDates = [
"2023-10-01T12:00:00Z",
"2023-10-02T14:30:00Z",
"2023-10-02T14:45:00Z",
"2023-10-07T23:59:59Z"
];

const heatmap = buildHourlyHeatmapFromCommitDates(commitDates);

expect(heatmap[0][12]).toBe(1);
expect(heatmap[1][14]).toBe(2);
expect(heatmap[6][23]).toBe(1);

// Check that other arbitrary cells are 0
expect(heatmap[0][0]).toBe(0);
expect(heatmap[3][12]).toBe(0);
Comment on lines +39 to +45

Choose a reason for hiding this comment

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

medium

Checking individual cells is a good start, but it doesn't guarantee that other cells weren't unintentionally modified. A more robust approach is to compare the entire resulting heatmap with a fully defined expected heatmap. This ensures the function produces the exact output required and has no side effects on other parts of the data structure.

        const expected = Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0));
        expected[0][12] = 1;
        expected[1][14] = 2;
        expected[6][23] = 1;
        expect(heatmap).toEqual(expected);

});
});

describe("getMostActiveHour", () => {
it("returns 0 for an all-zero heatmap", () => {
const heatmap = Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0));
expect(getMostActiveHour(heatmap)).toBe(0);
});

it("returns the correct most active hour", () => {
const heatmap = Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0));
heatmap[0][10] = 5; // Sunday 10am has 5
heatmap[1][10] = 3; // Monday 10am has 3 (Total for 10am: 8)

heatmap[2][15] = 10; // Tuesday 3pm has 10 (Total for 3pm: 10)

expect(getMostActiveHour(heatmap)).toBe(15);
});

it("returns the earliest hour in case of a tie", () => {
const heatmap = Array.from({ length: 7 }, () => Array.from({ length: 24 }, () => 0));
heatmap[0][5] = 10; // 5am has 10
heatmap[1][12] = 10; // 12pm has 10

// Since `if (total > maxCount)` is used, it should keep the first one it encounters (5am)
expect(getMostActiveHour(heatmap)).toBe(5);
});
});

describe("getMostActiveDayFromCalendar", () => {
it("returns 'Sunday' for an empty calendar", () => {
expect(getMostActiveDayFromCalendar([])).toBe("Sunday");
});

it("ignores entries with count <= 0", () => {
const calendar = [
{ date: "2023-10-01", count: 0 }, // Sunday
{ date: "2023-10-02", count: -5 }, // Monday
{ date: "2023-10-03", count: 1 } // Tuesday
];
expect(getMostActiveDayFromCalendar(calendar)).toBe("Tuesday");
});

it("returns the most active day", () => {
const calendar = [
{ date: "2023-10-01", count: 2 }, // Sunday
{ date: "2023-10-02", count: 10 }, // Monday
{ date: "2023-10-09", count: 5 }, // Another Monday (Total for Monday: 15)
{ date: "2023-10-06", count: 14 } // Friday (Total for Friday: 14)
];
expect(getMostActiveDayFromCalendar(calendar)).toBe("Monday");
});

it("returns the earliest day in the week in case of a tie", () => {
const calendar = [
{ date: "2023-10-03", count: 10 }, // Tuesday
{ date: "2023-10-05", count: 10 } // Thursday
];
// Since `if (totals[i] > totals[maxDay])` is used, the first max day (Tuesday) should be kept.
expect(getMostActiveDayFromCalendar(calendar)).toBe("Tuesday");
});
});
Loading