Fix getMeasurements(), which retrieves the invocation details for the…

… compare view (#195)

src/backend/compare/compare.ts

@@ -84,7 +84,10 @@ export async function getMeasurementsAsJson(
   completeRequestAndHandlePromise(start, db, 'get-measurements');
 }
 
-async function getMeasurements(
+/**
+ * Only exported for testing, meant to be private.
+ */
+export async function getMeasurements(
   projectSlug: string,
   runId: number,
   baseCommitId: string,
@@ -95,10 +98,10 @@ async function getMeasurements(
     name: 'fetchMeasurementsByProjectIdRunIdCommitId',
     text: `SELECT
               trialId, source.commitId as commitId,
-              invocation, iteration, warmup,
+              invocation, warmup,
               criterion.name as criterion,
               criterion.unit as unit,
-              value
+              values
             FROM
               Measurement
               JOIN Trial ON trialId = Trial.id
@@ -110,7 +113,7 @@ async function getMeasurements(
             WHERE Project.slug = $1
               AND runId = $2
               AND (source.commitId = $3 OR source.commitId = $4)
-            ORDER BY trialId, criterion, invocation, iteration;`,
+            ORDER BY trialId, criterion, invocation;`,
     values: [projectSlug, runId, baseCommitId, changeCommitId]
   };
   const result = await db.query(q);
@@ -151,7 +154,7 @@ async function getMeasurements(
         critObject.values[r.invocation - 1] = [];
       }
 
-      critObject.values[r.invocation - 1][r.iteration - 1] = r.value;
+      critObject.values[r.invocation - 1] = r.values;
     }
   }
 

src/frontend/plots.ts

@@ -1,4 +1,10 @@
-import type { AllResults, PlotData, TimelineResponse } from '../shared/api.js';
+import type {
+  AllResults,
+  CriterionWithoutData,
+  PlotData,
+  TimelineResponse,
+  ValuesPossiblyMissing
+} from '../shared/api.js';
 import type { Source } from '../backend/db/types.js';
 import { filterCommitMessage } from './render.js';
 import uPlot from '/static/uPlot.esm.min.js';
@@ -411,7 +417,7 @@ export function renderComparisonTimelinePlot(
 /**
  * Replace `0` with `null` to hide the point in the plot.
  */
-function replaceZeroByNull(data: number[]): number[] {
+function replaceZeroByNull(data: ValuesPossiblyMissing): ValuesPossiblyMissing {
   for (let i = 0; i < data.length; i += 1) {
     if (data[i] === 0) {
       (<any[]>data)[i] = null;
@@ -423,7 +429,7 @@ function replaceZeroByNull(data: number[]): number[] {
 
 function addSeries(
   critData: WarmupDataPerCriterion,
-  data: number[][],
+  data: (ValuesPossiblyMissing | CriterionWithoutData)[],
   series: any[],
   commitId: string,
   colors: readonly string[],
@@ -435,7 +441,14 @@ function addSeries(
         'Do not yet know how to handle multiple.'
     );
   }
-  data.push(replaceZeroByNull(critData.values[0]));
+
+  const values = critData.values[0];
+
+  if (values === null) {
+    return 0;
+  }
+
+  data.push(replaceZeroByNull(values));
 
   const style = styleMap[critData.criterion];
 
@@ -454,7 +467,7 @@ function addSeries(
     cfg.points.fill = colors[style.colorIdx];
   }
 
-  return critData.values[0].length;
+  return values.length;
 }
 
 function collectUnitsAndCriteria(data: WarmupDataForTrial[]): {

src/shared/view-types.ts

@@ -1,4 +1,9 @@
-import type { BenchmarkId, ProfileElement } from './api.js';
+import type {
+  BenchmarkId,
+  CriterionWithoutData,
+  ProfileElement,
+  ValuesPossiblyMissing
+} from './api.js';
 import type {
   CriterionData,
   Environment,
@@ -247,7 +252,7 @@ export type CompareView = CompareViewWithoutData | CompareViewWithData;
 export interface WarmupDataPerCriterion {
   criterion: string;
   unit: string;
-  values: number[][];
+  values: (ValuesPossiblyMissing | CriterionWithoutData)[];
 }
 
 export interface WarmupDataForTrial {

tests/backend/compare/db-measurements.test.ts

@@ -0,0 +1,44 @@
+import { describe, expect, it, beforeAll, afterAll } from '@jest/globals';
+
+import {
+  TestDatabase,
+  closeMainDb,
+  createAndInitializeDB
+} from '../db/db-testing.js';
+import { loadLargePayload } from '../../payload.js';
+import { getMeasurements } from '../../../src/backend/compare/compare.js';
+
+describe('compare view: getMeasurements()', () => {
+  let db: TestDatabase;
+
+  beforeAll(async () => {
+    db = await createAndInitializeDB('compare_view_get_m', 0, false);
+    const largeTestData = loadLargePayload();
+    await db.recordAllData(largeTestData);
+  });
+
+  afterAll(async () => {
+    if (db) return db.close();
+  });
+
+  it('should give expected results for runId 1', async () => {
+    const results = await getMeasurements(
+      'Large-Example-Project',
+      10,
+      '58666d1c84c652306f930daa72e7a47c58478e86',
+      '58666d1c84c652306f930daa72e7a47c58478e86',
+      db
+    );
+
+    expect(results).toHaveLength(1);
+    if (results === null) return;
+    expect(results[0].data).toHaveLength(1);
+    expect(results[0].data[0].criterion).toEqual('total');
+    expect(results[0].data[0].values).toHaveLength(1);
+    expect(results[0].data[0].values[0]).toHaveLength(1000);
+  });
+});
+
+afterAll(async () => {
+  return closeMainDb();
+});

tests/backend/db/db-setup.test.ts

@@ -22,7 +22,7 @@ import {
   closeMainDb
 } from './db-testing.js';
 import { robustPath } from '../../../src/backend/util.js';
-import { loadLargePayload } from '../../payload.js';
+import { loadLargePayload, loadSmallPayload } from '../../payload.js';
 
 const numTxStatements = 3;
 
@@ -71,9 +71,7 @@ describe('Recording a ReBench execution data fragments', () => {
   beforeAll(async () => {
     db = await createAndInitializeDB('db_setup');
 
-    basicTestData = JSON.parse(
-      readFileSync(robustPath('../tests/data/small-payload.json')).toString()
-    );
+    basicTestData = loadSmallPayload();
   });
 
   afterAll(async () => {
@@ -126,9 +124,7 @@ describe('Recording a ReBench execution data fragments', () => {
   });
 
   it('should accept trial denoise info', async () => {
-    const testData: BenchmarkData = JSON.parse(
-      readFileSync(robustPath('../tests/data/small-payload.json')).toString()
-    );
+    const testData = loadSmallPayload();
 
     const e = testData.env;
 
@@ -183,9 +179,7 @@ describe('Recording a ReBench execution from payload files', () => {
     // to access the database from R
     db = await createAndInitializeDB('db_setup_timeline', 25, true, false);
 
-    smallTestData = JSON.parse(
-      readFileSync(robustPath('../tests/data/small-payload.json')).toString()
-    );
+    smallTestData = loadSmallPayload();
     largeTestData = loadLargePayload();
     profileTestData = JSON.parse(
       readFileSync(robustPath('../tests/data/profile-payload.json')).toString()

tests/backend/db/db-testing.ts

@@ -15,6 +15,7 @@ import { Database } from '../../../src/backend/db/db.js';
 import { DatabaseWithPool } from '../../../src/backend/db/database-with-pool.js';
 // eslint-disable-next-line max-len
 import { BatchingTimelineUpdater } from '../../../src/backend/timeline/timeline-calc.js';
+import { reportConnectionRefused } from '../../../src/shared/errors.js';
 
 export class TestDatabase extends Database {
   private readonly connectionPool: Pool;
@@ -134,14 +135,23 @@ export async function createAndInitializeDB(
   timelineEnabled = false,
   useTransactions = true
 ): Promise<TestDatabase> {
-  const testDb = await createDB(
-    testSuite,
-    numBootstrapSamples,
-    timelineEnabled,
-    useTransactions
-  );
-  await testDb.prepareForTesting();
-  return testDb;
+  try {
+    const testDb = await createDB(
+      testSuite,
+      numBootstrapSamples,
+      timelineEnabled,
+      useTransactions
+    );
+    await testDb.prepareForTesting();
+    return testDb;
+  } catch (e: any) {
+    if (e.code == 'ECONNREFUSED') {
+      reportConnectionRefused(e);
+      throw new Error('Database connection refused');
+    }
+
+    throw e;
+  }
 }
 
 export async function createDB(

tests/backend/db/db.test.ts

@@ -7,7 +7,6 @@ import {
   it,
   jest
 } from '@jest/globals';
-import { readFileSync } from 'node:fs';
 import { PoolConfig, QueryConfig, QueryResult, QueryResultRow } from 'pg';
 
 import {
@@ -21,7 +20,6 @@ import {
   TimelineRequest
 } from '../../../src/shared/api.js';
 
-import { robustPath } from '../../../src/backend/util.js';
 import {
   Experiment,
   Environment,
@@ -30,6 +28,7 @@ import {
   Criterion
 } from '../../../src/backend/db/types.js';
 import { Database } from '../../../src/backend/db/db.js';
+import { loadSmallPayload } from '../../payload.js';
 
 describe('Record Trial', () => {
   let db: TestDatabase;
@@ -39,10 +38,7 @@ describe('Record Trial', () => {
 
   beforeAll(async () => {
     db = await createAndInitializeDB('db_record_trial', 0, false, false);
-    const data = readFileSync(
-      robustPath('../tests/data/small-payload.json')
-    ).toString();
-    basicTestData = JSON.parse(data);
+    basicTestData = loadSmallPayload();
 
     env = await db.recordEnvironment(basicTestData.env);
     exp = await db.recordExperiment(basicTestData);
@@ -109,10 +105,7 @@ describe('Timeline-plot Queries', () => {
   beforeAll(async () => {
     db = await createAndInitializeDB('db_ts_basic', 25, true, false);
 
-    const data = readFileSync(
-      robustPath('../tests/data/small-payload.json')
-    ).toString();
-    const basicTestData: BenchmarkData = JSON.parse(data);
+    const basicTestData = loadSmallPayload();
     projectName = basicTestData.projectName;
 
     baseBranch = basicTestData.source.branchOrTag = 'base-branch';

tests/backend/main/with-data.test.ts

@@ -1,13 +1,11 @@
 import { describe, expect, beforeAll, afterAll, it, jest } from '@jest/globals';
-import { readFileSync } from 'fs';
 
 import {
   TestDatabase,
   createAndInitializeDB,
   closeMainDb
 } from '../db/db-testing.js';
 
-import type { BenchmarkData } from '../../../src/shared/api.js';
 import { robustPath } from '../../../src/backend/util.js';
 import {
   getChanges,
@@ -20,6 +18,7 @@ import { prepareTemplate } from '../../../src/backend/templates.js';
 import { initJestMatchers } from '../../helpers.js';
 // eslint-disable-next-line max-len
 import { getLatestBenchmarksForTimelineView } from '../../../src/backend/timeline/timeline.js';
+import { loadSmallPayload } from '../../payload.js';
 
 initJestMatchers();
 
@@ -39,10 +38,7 @@ describe('Test with basic test data loaded', () => {
   beforeAll(async () => {
     db = await createAndInitializeDB('main_with_data', 25, true, false);
 
-    const data = readFileSync(
-      robustPath('../tests/data/small-payload.json')
-    ).toString();
-    const basicTestData: BenchmarkData = JSON.parse(data);
+    const basicTestData = loadSmallPayload();
     projectName = basicTestData.projectName;
 
     await db.recordAllData(basicTestData);

tests/backend/rebench/api.test.ts

@@ -5,7 +5,11 @@ import { ValidateFunction } from 'ajv';
 import { createValidator } from '../../../src/backend/rebench/api-validator.js';
 import { robustPath } from '../../../src/backend/util.js';
 import { assert, log } from '../../../src/backend/logging.js';
-import { loadLargePayload, loadLargePayloadApiV1 } from '../../payload.js';
+import {
+  loadLargePayload,
+  loadLargePayloadApiV1,
+  loadSmallPayload
+} from '../../payload.js';
 import type { BenchmarkData } from '../../../src/shared/api.js';
 import type { DataPointV1 } from '../../../src/backend/common/api-v1.js';
 
@@ -17,9 +21,7 @@ describe('Ensure Test Payloads conform to API', () => {
   });
 
   it('should validate small-payload.json', () => {
-    const testData = JSON.parse(
-      readFileSync(robustPath('../tests/data/small-payload.json')).toString()
-    );
+    const testData = loadSmallPayload();
 
     const result = validateFn(testData);
     if (!result) {

tests/payload.ts

@@ -9,6 +9,12 @@ import type {
 } from '../src/backend/db/types.js';
 import { assert } from '../src/backend/logging.js';
 
+export function loadSmallPayload(): BenchmarkData {
+  return JSON.parse(
+    readFileSync(robustPath('../tests/data/small-payload.json')).toString()
+  );
+}
+
 export function loadLargePayload(): BenchmarkData {
   const testData = JSON.parse(
     readFileSync(robustPath('../tests/data/large-payload.json')).toString()