-
Notifications
You must be signed in to change notification settings - Fork 0
Add CodSpeed performance benchmarks #51
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| name: CodSpeed | ||
|
|
||
| on: | ||
| push: | ||
| branches: | ||
| - main | ||
| pull_request: | ||
| # `workflow_dispatch` allows CodSpeed to trigger backtest | ||
| # performance analysis in order to generate initial data. | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: read | ||
| id-token: write | ||
|
|
||
| jobs: | ||
| benchmarks: | ||
| name: Run benchmarks | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-node@v4 | ||
| with: | ||
| node-version: 22 | ||
| cache: npm | ||
|
|
||
| - name: Install dependencies | ||
| run: npm ci | ||
|
|
||
| - name: Run benchmarks | ||
| uses: CodSpeedHQ/action@v4 | ||
|
Check warning on line 32 in .github/workflows/codspeed.yml
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| with: | ||
| mode: simulation | ||
| run: npx vitest bench --run | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,11 @@ | ||
| "use client"; | ||
|
|
||
| import { useState } from "react"; | ||
|
|
||
| const translations = { | ||
| en: { | ||
| title: "Coming Soon", | ||
| subtitle: "Something amazing is on the way", | ||
| language: "English", | ||
| }, | ||
| zh: { | ||
| title: "即将上线", | ||
| subtitle: "精彩内容即将呈现", | ||
| language: "中文", | ||
| }, | ||
| }; | ||
|
|
||
| type Language = keyof typeof translations; | ||
| import { getTranslation, type Language } from "@/lib/translations"; | ||
|
Check warning on line 4 in app/page.tsx
|
||
|
|
||
| export default function Home() { | ||
| const [lang, setLang] = useState<Language>("en"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type Safety Concern: Recommendation: const validLanguages: Language[] = ['en', 'zh'];
const selectedLang = e.target.value;
if (validLanguages.includes(selectedLang as Language)) {
setLang(selectedLang as Language);
}This ensures only valid languages are set. |
||
| const t = translations[lang]; | ||
| const t = getTranslation(lang); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential Error Handling Issue: Recommendation: const t = getTranslation(lang) || { title: 'Coming Soon', subtitle: '' };Or wrap in a try/catch if exceptions are possible. |
||
|
|
||
| return ( | ||
| <> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| import { bench, describe } from "vitest"; | ||
|
Check warning on line 1 in bench/games.bench.ts
|
||
| import { | ||
|
Check warning on line 2 in bench/games.bench.ts
|
||
| filterGamesByCategory, | ||
| sortGamesByTitle, | ||
| searchGames, | ||
|
Check notice on line 5 in bench/games.bench.ts
|
||
| getUniqueCategories, | ||
| paginateGames, | ||
| type Game, | ||
|
Check notice on line 8 in bench/games.bench.ts
|
||
| } from "../lib/games"; | ||
|
Check warning on line 9 in bench/games.bench.ts
|
||
| import { getTranslation, getAvailableLanguages } from "../lib/translations"; | ||
|
|
||
| // --- Test data --- | ||
|
|
||
| const categories = ["Action", "Puzzle", "Strategy", "RPG", "Sports", "Racing"]; | ||
|
|
||
| function generateGames(count: number): Game[] { | ||
| const games: Game[] = []; | ||
| for (let i = 0; i < count; i++) { | ||
| games.push({ | ||
| id: i, | ||
| title: `Game ${String(i).padStart(4, "0")}`, | ||
| description: `An exciting ${categories[i % categories.length].toLowerCase()} game with unique mechanics and challenging levels`, | ||
| image: `/images/game-${i}.jpg`, | ||
| category: categories[i % categories.length], | ||
| }); | ||
| } | ||
| return games; | ||
| } | ||
|
|
||
| const smallCatalog = generateGames(50); | ||
| const largeCatalog = generateGames(1000); | ||
|
|
||
| // --- Benchmarks --- | ||
|
|
||
| describe("translations", () => { | ||
| bench("getTranslation - existing language", () => { | ||
| getTranslation("en"); | ||
| }); | ||
|
|
||
| bench("getTranslation - fallback to default", () => { | ||
| getTranslation("fr"); | ||
| }); | ||
|
|
||
| bench("getAvailableLanguages", () => { | ||
| getAvailableLanguages(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("game catalog - small (50 games)", () => { | ||
| bench("filterGamesByCategory", () => { | ||
| filterGamesByCategory(smallCatalog, "Action"); | ||
| }); | ||
|
|
||
| bench("sortGamesByTitle", () => { | ||
| sortGamesByTitle(smallCatalog); | ||
| }); | ||
|
|
||
| bench("searchGames", () => { | ||
| searchGames(smallCatalog, "unique mechanics"); | ||
| }); | ||
|
|
||
| bench("getUniqueCategories", () => { | ||
| getUniqueCategories(smallCatalog); | ||
| }); | ||
|
|
||
| bench("paginateGames", () => { | ||
| paginateGames(smallCatalog, 2, 10); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| }); | ||
|
|
||
| describe("game catalog - large (1000 games)", () => { | ||
| bench("filterGamesByCategory", () => { | ||
| filterGamesByCategory(largeCatalog, "Strategy"); | ||
| }); | ||
|
|
||
| bench("sortGamesByTitle", () => { | ||
| sortGamesByTitle(largeCatalog); | ||
| }); | ||
|
|
||
| bench("searchGames", () => { | ||
| searchGames(largeCatalog, "unique mechanics"); | ||
| }); | ||
|
|
||
| bench("getUniqueCategories", () => { | ||
| getUniqueCategories(largeCatalog); | ||
| }); | ||
|
|
||
| bench("paginateGames", () => { | ||
| paginateGames(largeCatalog, 5, 20); | ||
| }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| }); | ||
|
Comment on lines
+35
to
+91
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The benchmarks currently do not validate the correctness of the results returned by the functions under test. This means that if a function's implementation changes and produces incorrect results, the benchmarks will not detect it. Consider adding assertions to verify that the output is as expected, even in performance tests. For example: bench("filterGamesByCategory", () => {
const result = filterGamesByCategory(smallCatalog, "Action");
if (!Array.isArray(result) || result.some(g => g.category !== "Action")) {
throw new Error("filterGamesByCategory returned incorrect results");
}
});This ensures that performance measurements are only taken for correct implementations. |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,37 @@ | ||||||||||||||||||||||||||
| export interface Game { | ||||||||||||||||||||||||||
| id: number; | ||||||||||||||||||||||||||
| title: string; | ||||||||||||||||||||||||||
| description: string; | ||||||||||||||||||||||||||
| image: string; | ||||||||||||||||||||||||||
| category: string; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+1
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 | Confidence: High The Code Suggestion: // components/GameCard.tsx
import type { Game } from "@/lib/games";
interface GameCardProps {
game: Game;
}Evidence: path:components/GameCard.tsx |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function filterGamesByCategory(games: Game[], category: string): Game[] { | ||||||||||||||||||||||||||
| return games.filter((game) => game.category === category); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function sortGamesByTitle(games: Game[]): Game[] { | ||||||||||||||||||||||||||
| return [...games].sort((a, b) => a.title.localeCompare(b.title)); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function searchGames(games: Game[], query: string): Game[] { | ||||||||||||||||||||||||||
| const lowerQuery = query.toLowerCase(); | ||||||||||||||||||||||||||
| return games.filter( | ||||||||||||||||||||||||||
| (game) => | ||||||||||||||||||||||||||
| game.title.toLowerCase().includes(lowerQuery) || | ||||||||||||||||||||||||||
| game.description.toLowerCase().includes(lowerQuery), | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+17
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚩 searchGames returns all games when query is empty string
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function getUniqueCategories(games: Game[]): string[] { | ||||||||||||||||||||||||||
| return [...new Set(games.map((game) => game.category))].sort(); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| export function paginateGames( | ||||||||||||||||||||||||||
| games: Game[], | ||||||||||||||||||||||||||
| page: number, | ||||||||||||||||||||||||||
| pageSize: number, | ||||||||||||||||||||||||||
| ): Game[] { | ||||||||||||||||||||||||||
| const start = (page - 1) * pageSize; | ||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Quality - The "paginateGames" function fails to validate "page" and "pageSize", causing incorrect array slicing, such as negative start indexes when "page=0". This misaligns the control flow from its intended pagination logic. View in Corgea ↗ More Details🎟️Issue Explanation: The "paginateGames" function fails to validate "page" and "pageSize", causing incorrect array slicing, such as negative start indexes when "page=0". This misaligns the control flow from its intended pagination logic. - Negative or zero "page" causes "start" to be negative, making "slice" return unexpected results by counting from the array's end. - Passing non-integer or negative "pageSize" leads to inconsistent slicing behavior, breaking assumptions in pagination. - This flaw increases debugging complexity and reduces reliability, as pagination outputs don't align with expected page boundaries. 🪄Fix Explanation: Added input validation and normalization for pagination parameters to prevent unexpected behavior and ensure the function only processes valid positive integers. Input validation with "Number.isFinite" ensures that non-numeric or infinite values immediately return an empty array, preventing runtime errors. Flooring page and pageSize via "Math.floor" normalizes fractional inputs, improving consistency and expected pagination behavior. Checks for positive values ("safePage < 1 || safePageSize < 1") guard against invalid pagination requests like zero or negative numbers. Calculating "start" with validated and normalized inputs guarantees the slice range is always valid, improving robustness. 💡Important Instructions: Audit other pagination or indexing functions to apply similar input validation patterns for consistent robustness across the codebase.
Suggested change
|
||||||||||||||||||||||||||
| return games.slice(start, start + pageSize); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
|
Comment on lines
+30
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lack of input validation in The Recommended solution: if (page < 1 || pageSize < 1) {
throw new Error('Page and pageSize must be positive integers');
}
Comment on lines
+30
to
+37
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: paginateGames uses 1-based page indexing without validation
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,25 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export const translations: Record< | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| string, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+2
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. issue: The explicit Because You can keep the literal key union by relying on inference and export const translations = {
en: { ... },
zh: { ... },
} as const satisfies Record<string, { title: string; subtitle: string; language: string }>;
export type Language = keyof typeof translations; // 'en' | 'zh'Then |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| { title: string; subtitle: string; language: string } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| > = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| en: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| title: "Coming Soon", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subtitle: "Something amazing is on the way", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| language: "English", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| zh: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| title: "即将上线", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| subtitle: "精彩内容即将呈现", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| language: "中文", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+15
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: translations object is mutable and could be modified at runtime The Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export type Language = keyof typeof translations; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+1
to
+17
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Exported The
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function getTranslation(lang: string) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return translations[lang] ?? translations["en"]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+19
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logic & Type Safety: export function getTranslation(lang: Language) {
return translations[lang];
}Or, if you must accept any string, validate it: export function getTranslation(lang: string) {
if (lang in translations) {
return translations[lang];
}
return translations["en"];
}
Comment on lines
+17
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: The Since you already expose a type Translation = (typeof translations)[Language];
export function getTranslation(lang: Language | string): Translation {
return translations[lang as Language] ?? translations.en;
}This keeps
Suggested change
Comment on lines
+19
to
+21
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📝 Info: getTranslation fallback is safe at runtime despite type widening Despite the Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+19
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 | Confidence: High The
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function getAvailableLanguages(): string[] { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Returning If export function getAvailableLanguages(): Language[] {
return Object.keys(translations) as Language[];
}This tightens the type so consumers can’t pass arbitrary strings where only supported languages are allowed. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Object.keys(translations); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type Consistency: export function getAvailableLanguages(): Language[] {
return Object.keys(translations) as Language[];
} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
credential persistence through GitHub Actions artifacts [zizmor:zizmor/artipacked]