better organization

ankitml · ankitml · commit 15b062829d5c · 2025-12-26T20:19:50.000+05:30
diff --git a/src/addBenchmarkEntry.ts b/src/addBenchmarkEntry.ts
@@ -21,7 +21,7 @@ export function addBenchmarkEntry(
     } else {
         const suites = entries[benchName];
 
-        // Use git-graph aware logic to find previous benchmark
+        // Find previous benchmark using git ancestry
         core.debug(`Finding previous benchmark for commit: ${benchEntry.commit.id}`);
 
         prevBench = gitAnalyzer.findPreviousBenchmark(suites, benchEntry.commit.id);
diff --git a/src/config.ts b/src/config.ts
@@ -4,6 +4,7 @@ import * as os from 'os';
 import * as path from 'path';
 
 export type ToolType = typeof VALID_TOOLS[number];
+
 export interface Config {
     name: string;
     tool: ToolType;
diff --git a/src/default_index_html.ts b/src/default_index_html.ts
@@ -127,19 +127,6 @@ export const DEFAULT_INDEX_HTML = String.raw`<!DOCTYPE html>
         };
 
         function init() {
-          // By default, we use the order provided by the JSON data (which is sorted by git topology)
-          // We disable client-side timestamp sorting to avoid breaking the topological order
-          // Future work: make this configurable via a JSON flag
-          function sortEntriesByTimestamp(entries) {
-            // Sort benchmarks by commit timestamp instead of execution time
-            // This provides better git graph ordering for visualization
-            return [...entries].sort((a, b) => {
-              const timestampA = new Date(a.commit.timestamp).getTime();
-              const timestampB = new Date(b.commit.timestamp).getTime();
-              return timestampA - timestampB;
-            });
-          }
-
           function collectBenchesPerTestCase(entries) {
             const map = new Map();
             for (const entry of entries) {
@@ -154,17 +141,6 @@ export const DEFAULT_INDEX_HTML = String.raw`<!DOCTYPE html>
                 }
               }
             }
-            // Sort each benchmark's data points by commit timestamp to ensure consistent ordering
-            // DISABLED: strictly use server-side order
-            /*
-            for (const [benchName, arr] of map.entries()) {
-              arr.sort((a, b) => {
-                const timestampA = new Date(a.commit.timestamp).getTime();
-                const timestampB = new Date(b.commit.timestamp).getTime();
-                return timestampA - timestampB;
-              });
-            }
-            */
             return map;
           }
 
@@ -185,14 +161,12 @@ export const DEFAULT_INDEX_HTML = String.raw`<!DOCTYPE html>
             a.click();
           };
 
-          // Prepare data points for charts
+          // Prepare data points for charts (uses server-side ordering)
           return Object.keys(data.entries).map(name => {
             const entries = data.entries[name];
-            // const sortedEntries = sortEntriesByTimestamp(entries);
-            const sortedEntries = entries;
             return {
               name,
-              dataSet: collectBenchesPerTestCase(sortedEntries),
+              dataSet: collectBenchesPerTestCase(entries),
             };
           });
         }
diff --git a/src/gitGraph.ts b/src/gitGraph.ts
@@ -1,4 +1,3 @@
-import * as github from '@actions/github';
 import { execSync } from 'child_process';
 import * as core from '@actions/core';
 import { Benchmark } from './extract';
@@ -16,46 +15,16 @@ export class GitGraphAnalyzer {
     }
 
     /**
-     * Get current branch from GitHub context
+     * Check if git CLI is available
      */
-    getCurrentBranch(): string {
-        const context = github.context;
-
-        // For pull requests, get the head branch
-        if (context.payload.pull_request) {
-            return context.payload.pull_request.head.ref;
-        }
-
-        // Try to get branch from git CLI first if available
-        if (this.gitCliAvailable) {
-            try {
-                const branch = execSync('git rev-parse --abbrev-ref HEAD', {
-                    encoding: 'utf8',
-                    cwd: process.env.GITHUB_WORKSPACE ?? process.cwd(),
-                }).trim();
-
-                if (branch && branch !== 'HEAD') {
-                    return branch;
-                }
-            } catch (e) {
-                core.debug(`Failed to get branch from git CLI: ${e}`);
-            }
-        }
-
-        // For pushes, get the branch from ref
-        if (context.ref) {
-            // Remove 'refs/heads/' prefix if present
-            return context.ref.replace('refs/heads/', '');
-        }
-
-        // Fallback to 'main' if we can't determine branch
-        return 'main';
+    isGitAvailable(): boolean {
+        return this.gitCliAvailable;
     }
 
     /**
-     * Get git ancestry using topological order (only works in GitHub Actions environment)
+     * Get git ancestry using topological order
      */
-    getBranchAncestry(ref: string): string[] {
+    getAncestry(ref: string): string[] {
         if (!this.gitCliAvailable) {
             core.warning('Git CLI not available, cannot determine ancestry');
             return [];
@@ -78,10 +47,11 @@ export class GitGraphAnalyzer {
     }
 
     /**
-     * Find previous benchmark commit based on git graph structure
+     * Find previous benchmark commit based on git ancestry.
+     * Falls back to execution time ordering if git ancestry is not available.
      */
     findPreviousBenchmark(suites: Benchmark[], currentSha: string): Benchmark | null {
-        const ancestry = this.getBranchAncestry(currentSha);
+        const ancestry = this.getAncestry(currentSha);
 
         if (ancestry.length === 0) {
             core.warning(`No ancestry found for commit ${currentSha}, falling back to execution time ordering`);
@@ -111,37 +81,16 @@ export class GitGraphAnalyzer {
         return null;
     }
 
-    /**
-     * Sort benchmark data by commit timestamp (for GitHub Pages visualization)
-     * This doesn't need git CLI - just uses the commit timestamps already stored
-     */
-    sortByGitOrder(suites: Benchmark[]): Benchmark[] {
-        if (suites.length === 0) return suites;
-
-        // For GitHub Pages, we don't have git CLI, so sort by commit timestamp
-        // This gives a reasonable approximation of git order
-        const sortedSuites = [...suites].sort((a, b) => {
-            const timestampA = new Date(a.commit.timestamp ?? '1970-01-01T00:00:00Z').getTime();
-            const timestampB = new Date(b.commit.timestamp ?? '1970-01-01T00:00:00Z').getTime();
-            return timestampA - timestampB;
-        });
-
-        core.debug('Sorted benchmarks by commit timestamp (GitHub Pages mode)');
-        return sortedSuites;
-    }
-
     /**
      * Find the insertion index for a new benchmark entry based on git ancestry.
-     * Returns the index after which the new entry should be inserted.
-     * If no ancestor is found, returns -1 (insert at beginning) or suites.length (append to end).
+     * Inserts after the most recent ancestor in the existing suites.
      */
     findInsertionIndex(suites: Benchmark[], newCommitSha: string): number {
         if (!this.gitCliAvailable || suites.length === 0) {
-            // Fallback: append to end
             return suites.length;
         }
 
-        const ancestry = this.getBranchAncestry(newCommitSha);
+        const ancestry = this.getAncestry(newCommitSha);
         if (ancestry.length === 0) {
             core.debug('No ancestry found, appending to end');
             return suites.length;
diff --git a/src/write.ts b/src/write.ts
@@ -490,7 +490,7 @@ async function writeBenchmarkToExternalJson(
     jsonFilePath: string,
     config: Config,
 ): Promise<{ prevBench: Benchmark | null; normalizedCurrentBench: Benchmark }> {
-    const { name, maxItemsInChart, saveDataFile } = config;
+    const { name, saveDataFile, maxItemsInChart } = config;
     const data = await loadDataJson(jsonFilePath);
     const { prevBench, normalizedCurrentBench } = addBenchmarkToDataJson(name, bench, data, maxItemsInChart);
 
diff --git a/test/addBenchmarkEntryGitGraph.test.ts b/test/addBenchmarkEntryGitGraph.test.ts
@@ -1,6 +1,5 @@
 // Mock modules before imports
 const mockDebug = jest.fn();
-const mockGetCurrentBranch = jest.fn();
 const mockFindPreviousBenchmark = jest.fn();
 const mockFindInsertionIndex = jest.fn();
 
@@ -10,7 +9,6 @@ jest.mock('@actions/core', () => ({
 
 jest.mock('../src/gitGraph', () => ({
     GitGraphAnalyzer: jest.fn().mockImplementation(() => ({
-        getCurrentBranch: mockGetCurrentBranch,
         findPreviousBenchmark: mockFindPreviousBenchmark,
         findInsertionIndex: mockFindInsertionIndex,
     })),
@@ -52,16 +50,12 @@ describe('addBenchmarkEntry with Git Graph', () => {
             [benchName]: [existingEntry],
         };
 
-        mockGetCurrentBranch.mockReturnValue('feature-branch');
         mockFindPreviousBenchmark.mockReturnValue(existingEntry);
         mockFindInsertionIndex.mockReturnValue(1);
 
         const result = addBenchmarkEntry(benchName, benchEntry, entries, null);
 
-        expect(mockFindPreviousBenchmark).toHaveBeenCalledWith(
-            expect.arrayContaining([existingEntry]),
-            'abc123',
-        );
+        expect(mockFindPreviousBenchmark).toHaveBeenCalledWith(expect.arrayContaining([existingEntry]), 'abc123');
         expect(mockDebug).toHaveBeenCalledWith('Finding previous benchmark for commit: abc123');
         expect(mockDebug).toHaveBeenCalledWith('Found previous benchmark: def456');
         expect(result.prevBench).toBe(existingEntry);
@@ -75,7 +69,6 @@ describe('addBenchmarkEntry with Git Graph', () => {
             [benchName]: [existingEntry],
         };
 
-        mockGetCurrentBranch.mockReturnValue('main');
         mockFindPreviousBenchmark.mockReturnValue(null);
         mockFindInsertionIndex.mockReturnValue(1);
 
@@ -91,7 +84,6 @@ describe('addBenchmarkEntry with Git Graph', () => {
         const benchEntry = createMockBenchmark('abc123');
         const entries: any = {};
 
-        mockGetCurrentBranch.mockReturnValue('main');
         mockFindPreviousBenchmark.mockReturnValue(null);
 
         const result = addBenchmarkEntry(benchName, benchEntry, entries, null);
@@ -112,7 +104,6 @@ describe('addBenchmarkEntry with Git Graph', () => {
             [benchName]: [existingEntry],
         };
 
-        mockGetCurrentBranch.mockReturnValue('main');
         mockFindPreviousBenchmark.mockReturnValue(existingEntry);
         mockFindInsertionIndex.mockReturnValue(1);
 
@@ -136,7 +127,6 @@ describe('addBenchmarkEntry with Git Graph', () => {
             [benchName]: oldEntries,
         };
 
-        mockGetCurrentBranch.mockReturnValue('main');
         mockFindPreviousBenchmark.mockReturnValue(oldEntries[oldEntries.length - 1]);
         mockFindInsertionIndex.mockReturnValue(3);
 
@@ -168,7 +158,6 @@ describe('addBenchmarkEntry with Git Graph', () => {
             [benchName]: oldEntries,
         };
 
-        mockGetCurrentBranch.mockReturnValue('main');
         mockFindPreviousBenchmark.mockReturnValue(oldEntries[0]);
         mockFindInsertionIndex.mockReturnValue(1);
 
diff --git a/test/gitGraphAnalyzer.test.ts b/test/gitGraphAnalyzer.test.ts
diff --git a/test/write.spec.ts b/test/write.spec.ts
