Scoped library payloads: load the track list when a view needs it #287

Open
yonlu wants to merge 10 commits from perf/279-284-scoped-library-payloads into main
pull from: perf/279-284-scoped-library-payloads
8 changed files with 149 additions and 46 deletions
Showing only changes of commit 655434dcaa - Show all commits

No files matched your search

+27
View File
@@ -0,0 +1,27 @@
import { chromium } from '@playwright/test';
const b = await chromium.launch({ executablePath: '/usr/bin/chromium' });
const p = await b.newPage();
await p.addInitScript({ path: '/mnt/vault/dev/golang/yellowjacket/.playwright/init-events.js' });
await p.goto('http://localhost:34115', { waitUntil: 'load' });
await p.evaluate(() => window.__yjEvents.ready(30000));
const out = await p.evaluate(async () => {
const time = async (label, path, args, n = 5) => {
const ms = [];
for (let i = 0; i < n; i++) {
const t0 = performance.now();
await window.__yjEvents.call(path, args, 60000);
ms.push(performance.now() - t0);
}
ms.sort((a, b) => a - b);
return { label, medianMs: Math.round(ms[Math.floor(n / 2)]), all: ms.map((m) => Math.round(m)) };
};
return [
await time('GetTrackTable(0)', 'library.Library.GetTrackTable', [0], 4),
await time('GetAlbums(0)', 'library.Library.GetAlbums', [0], 4),
await time('SearchLocal(tide)', 'explore.Service.SearchLocal', ['tide'], 4),
await time('SearchLocal(rock)', 'explore.Service.SearchLocal', ['rock'], 4),
await time('SearchTracks(tide)', 'library.Library.SearchTracks', ['tide', 0], 4),
];
});
console.log(JSON.stringify(out, null, 1));
await b.close();
+80
View File
@@ -0,0 +1,80 @@
/**
* Every bound method the suite names by hand must exist.
*
* A binding call carries only a method id, so a spec that names a method
* the Go side no longer has fails at *runtime*, in whichever spec
* happens to call it, with `unknown bound method name` — and nothing
* before that. `library.Library.GetTracks` was replaced by
* `GetTrackTable` (#281) and four specs kept calling the old name: the
* suite reported twenty failures across transport, bottom-bar and
* reduced-motion specs, none of which mention the library list.
*
* The known names come from `support/method-ids.mjs`, which derives them
* from the generated bindings tree that `make bindings-check` keeps
* current — so this cannot disagree with what the app can answer.
*
* It asserts first that it read something: a glob that matched nothing
* would pass over an empty list.
*/
import { readFileSync, readdirSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { dirname, join } from 'node:path';
import { test, expect } from '../support/fixtures.js';
import { methodIDs } from '../support/method-ids.mjs';
const here = dirname(fileURLToPath(import.meta.url));
/** The specs, and the harness that calls bindings on their behalf. */
const DIRS = [here, join(here, '..', 'support')];
function sources(): Array<[string, string]> {
const out: Array<[string, string]> = [];
for (const dir of DIRS) {
for (const entry of readdirSync(dir)) {
if (!entry.endsWith('.ts')) continue;
if (entry.endsWith('.spec.ts') && entry === 'binding-names.spec.ts') continue;
const path = join(dir, entry);
out.push([path, readFileSync(path, 'utf8')]);
}
}
return out;
}
/** `'library.Library.GetTrackTable'` — package, service, method. */
const NAMED_BINDING = /'([a-z][A-Za-z0-9]*\.[A-Z]\w*\.[A-Z]\w*)'/g;
/**
* Names that are deliberately not bindings.
*
* `harness.spec.ts` calls an unknown method on purpose, to assert that
* the bridge rejects with a ReferenceError naming it rather than
* hanging — that spec is the reason a bad call is loud.
*/
const DELIBERATE = new Set(['queue.Queue.Nope']);
test('every binding named by a spec exists', () => {
// `methodIDs()` is the id -> name map the recorder names calls with.
const known = new Set(methodIDs().values());
const files = sources();
// A sweep over an empty glob passes and proves nothing.
expect(files.length, 'specs and support files read').toBeGreaterThan(20);
expect(known.size, 'bound method names derived').toBeGreaterThan(100);
const unknown: string[] = [];
for (const [path, source] of files) {
for (const [, name] of source.matchAll(NAMED_BINDING)) {
if (known.has(name!) || DELIBERATE.has(name!)) continue;
unknown.push(`${name} (${path.split('/').pop()})`);
}
}
expect([...new Set(unknown)]).toEqual([]);
});
+8 -10
View File
@@ -1,4 +1,10 @@
import { test, expect, callBinding, NO_QUEUE_SOURCE } from '../support/fixtures.js';
import {
test,
expect,
callBinding,
libraryTracks,
NO_QUEUE_SOURCE,
} from '../support/fixtures.js';
import type { Page } from '@playwright/test';
/**
@@ -39,15 +45,7 @@ const geometry = (app: Page) =>
/** Something has to be playing before the transport draws a seek bar. */
async function play(app: Page): Promise<void> {
const paths = await app.evaluate(async () => {
const tracks = (await window.__yjEvents.call(
'library.Library.GetTracks',
[0],
10_000,
)) as { FilePath: string }[];
return tracks.slice(0, 3).map((t) => t.FilePath);
});
const paths = (await libraryTracks(app)).slice(0, 3).map((t) => t.FilePath);
await callBinding(app, 'queue.Queue.SetQueue', [
paths,
+6 -12
View File
@@ -1,6 +1,7 @@
import {
test,
expect,
libraryTracks,
callBinding,
openTheQueue,
NO_QUEUE_SOURCE,
@@ -56,18 +57,11 @@ async function menuLabels(app: Page): Promise<string[]> {
* the wrong "no link".
*/
async function queueThree(app: Page): Promise<void> {
const paths = await app.evaluate(async () => {
const tracks = (await window.__yjEvents.call(
'library.Library.GetTracks',
[0],
10_000,
)) as { FilePath: string; Album: string; ArtistName: string }[];
return tracks
.filter((t) => t.Album !== '' && t.ArtistName !== '')
.slice(0, 3)
.map((t) => t.FilePath);
});
const tracks = await libraryTracks(app);
const paths = tracks
.filter((t) => t.Album !== '' && t.ArtistName !== '')
.slice(0, 3)
.map((t) => t.FilePath);
await callBinding(app, 'queue.Queue.SetQueue', [
paths,
+5 -9
View File
@@ -1,4 +1,4 @@
import { test, expect } from '../support/fixtures.js';
import { test, expect, libraryTracks } from '../support/fixtures.js';
/**
* The phone's transport (#59, #56).
@@ -64,19 +64,15 @@ async function sizeOf(
/** Put something in the queue, so the transport has a track to act on. */
async function stageATrack(page: Page): Promise<void> {
await page.evaluate(async () => {
const tracks = (await window.__yjEvents.call(
'library.Library.GetTracks',
[0],
10_000,
)) as { FilePath: string }[];
const paths = (await libraryTracks(page)).slice(0, 4).map((t) => t.FilePath);
await page.evaluate(async (paths) => {
await window.__yjEvents.call(
'queue.Queue.SetQueue',
[tracks.slice(0, 4).map((t) => t.FilePath), 0, false, { type: '', id: 0, label: '' }],
[paths, 0, false, { type: '', id: 0, label: '' }],
10_000,
);
});
}, paths);
}
test.describe('the phone bar carries three controls', () => {
+16 -1
View File
@@ -40,6 +40,21 @@ const FINISH_TIMEOUT = 60_000;
const libraryCalls = async (app: Page): Promise<string[]> =>
(await bindingCalls(app)).filter((c) => c.startsWith('library.Library.'));
/**
* The bindings that fetch a collection.
*
* A prefix test (GetAll*) used to stand in for this, and matched
* exactly one name — GetAllLibrariesWithTrackCounts — so the assertion
* below held whatever the app refetched. The sweep in
* binding-names.spec.ts is what found it.
*/
const COLLECTION_FETCHES = [
'library.Library.GetTrackTable',
'library.Library.GetAlbums',
'library.Library.GetArtists',
'library.Library.GetGenres',
];
/**
* Select rows by dispatching on the row rather than clicking it.
*
@@ -116,7 +131,7 @@ test.describe('a finished track', () => {
const refetched = await libraryCalls(app);
expect(
refetched.filter((c) => c.startsWith('library.Library.GetAll')),
refetched.filter((c) => COLLECTION_FETCHES.includes(c)),
'a play refetched a collection',
).toEqual([]);
+4 -14
View File
@@ -2,6 +2,7 @@ import {
test,
expect,
callBinding,
libraryTracks,
waitForEvent,
NO_QUEUE_SOURCE,
} from '../support/fixtures.js';
@@ -56,20 +57,9 @@ async function playTheLongOne(app: Page): Promise<void> {
window.dispatchEvent(new CustomEvent('yj-scroll-mode-changed'));
});
const paths: string[] = await app.evaluate(async (needle) => {
// One argument, and 0 means every library: the scoped and
// unscoped list queries collapsed into one when the schema did
// (plan 013 R3), so `GetTracks()` no longer exists to call.
const tracks = await window.__yjEvents.call(
'library.Library.GetTracks',
[0],
10_000,
);
return (tracks as { TrackName: string; FilePath: string }[])
.filter((t) => t.TrackName.startsWith(needle))
.map((t) => t.FilePath);
}, LONG_TITLE);
const paths: string[] = (await libraryTracks(app))
.filter((t) => t.TrackName.startsWith(LONG_TITLE))
.map((t) => t.FilePath);
expect(paths.length).toBeGreaterThan(0);
+3
View File
@@ -100,6 +100,7 @@ export async function callBinding<T = unknown>(
export interface LibraryTrack {
FilePath: string;
TrackName: string;
ArtistName: string;
Album: string;
}
@@ -116,12 +117,14 @@ export async function libraryTracks(page: Page): Promise<LibraryTrack[]> {
strings: string[];
filePath: string[];
trackName: number[];
artistName: number[];
album: number[];
}>(page, 'library.Library.GetTrackTable', [0]);
return (t.filePath ?? []).map((FilePath, i) => ({
FilePath,
TrackName: t.strings[t.trackName[i]!] ?? '',
ArtistName: t.strings[t.artistName[i]!] ?? '',
Album: t.strings[t.album[i]!] ?? '',
}));
}