From 5336ba496f83c8544f7e4e54392ae39acdf939fa Mon Sep 17 00:00:00 2001 From: Stefan Marr Date: Sun, 24 Mar 2024 20:56:08 +0000 Subject: [PATCH 1/4] Define loadSmallPayload() for consistency Signed-off-by: Stefan Marr --- tests/backend/db/db-setup.test.ts | 14 ++++---------- tests/backend/db/db.test.ts | 13 +++---------- tests/backend/main/with-data.test.ts | 8 ++------ tests/backend/rebench/api.test.ts | 10 ++++++---- tests/payload.ts | 6 ++++++ 5 files changed, 21 insertions(+), 30 deletions(-) diff --git a/tests/backend/db/db-setup.test.ts b/tests/backend/db/db-setup.test.ts index 644eb891..9c5693ff 100644 --- a/tests/backend/db/db-setup.test.ts +++ b/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() diff --git a/tests/backend/db/db.test.ts b/tests/backend/db/db.test.ts index 4b685cd5..096fc626 100644 --- a/tests/backend/db/db.test.ts +++ b/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'; diff --git a/tests/backend/main/with-data.test.ts b/tests/backend/main/with-data.test.ts index 93f526e2..73de0957 100644 --- a/tests/backend/main/with-data.test.ts +++ b/tests/backend/main/with-data.test.ts @@ -1,5 +1,4 @@ import { describe, expect, beforeAll, afterAll, it, jest } from '@jest/globals'; -import { readFileSync } from 'fs'; import { TestDatabase, @@ -7,7 +6,6 @@ import { 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); diff --git a/tests/backend/rebench/api.test.ts b/tests/backend/rebench/api.test.ts index 0efb187b..ecde5c3d 100644 --- a/tests/backend/rebench/api.test.ts +++ b/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) { diff --git a/tests/payload.ts b/tests/payload.ts index 2e9c4bfa..a8ef9e5e 100644 --- a/tests/payload.ts +++ b/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() From bce9857f27077d6099a30d4a91b05c0821a45ec3 Mon Sep 17 00:00:00 2001 From: Stefan Marr Date: Sun, 24 Mar 2024 22:38:08 +0000 Subject: [PATCH 2/4] Make connection-refused error more useful in tests Signed-off-by: Stefan Marr --- tests/backend/db/db-testing.ts | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/tests/backend/db/db-testing.ts b/tests/backend/db/db-testing.ts index 07693cd0..c35ee578 100644 --- a/tests/backend/db/db-testing.ts +++ b/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 { - 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( From db62d16ecf8e82fe17c5ef0d1c61b3c38ae3c975 Mon Sep 17 00:00:00 2001 From: Stefan Marr Date: Sun, 24 Mar 2024 22:58:26 +0000 Subject: [PATCH 3/4] Added test for getMeasurements() of compare.ts Signed-off-by: Stefan Marr --- src/backend/compare/compare.ts | 5 ++- tests/backend/compare/db-measurements.test.ts | 44 +++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 tests/backend/compare/db-measurements.test.ts diff --git a/src/backend/compare/compare.ts b/src/backend/compare/compare.ts index 4857ec52..3103f48e 100644 --- a/src/backend/compare/compare.ts +++ b/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, diff --git a/tests/backend/compare/db-measurements.test.ts b/tests/backend/compare/db-measurements.test.ts new file mode 100644 index 00000000..39d78812 --- /dev/null +++ b/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(); +}); From 0e83eaeb8bbbdcae6bec84856d12b45a0c3311bb Mon Sep 17 00:00:00 2001 From: Stefan Marr Date: Sun, 24 Mar 2024 22:58:51 +0000 Subject: [PATCH 4/4] Fix fetchMeasurementsByProjectIdRunIdCommitId query and related typing Signed-off-by: Stefan Marr --- src/backend/compare/compare.ts | 8 ++++---- src/frontend/plots.ts | 23 ++++++++++++++++++----- src/shared/view-types.ts | 9 +++++++-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/backend/compare/compare.ts b/src/backend/compare/compare.ts index 3103f48e..db4f23fa 100644 --- a/src/backend/compare/compare.ts +++ b/src/backend/compare/compare.ts @@ -98,10 +98,10 @@ export 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 @@ -113,7 +113,7 @@ export 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); @@ -154,7 +154,7 @@ export 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; } } diff --git a/src/frontend/plots.ts b/src/frontend/plots.ts index a082f0e0..cb54c7dc 100644 --- a/src/frontend/plots.ts +++ b/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) { (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[]): { diff --git a/src/shared/view-types.ts b/src/shared/view-types.ts index 40ee8c4d..2f35729b 100644 --- a/src/shared/view-types.ts +++ b/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 {