Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ jobs:
yarn lint
yarn test ci

- name: Run compat tests
run: |
yarn test-tape-compat
yarn test-vitest-smoke

- name: Coveralls
uses: coverallsapp/github-action@648a8eb78e6d50909eff900e4ec85cab4524a45b # v2.3.6
with:
Expand Down
3 changes: 2 additions & 1 deletion .ocularrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ const config = {
'test-browser': 'index.html',
bench: 'test/bench/index.js',
'bench-browser': 'test/bench/browser.html',
size: 'test/size/import-nothing.js'
size: 'test/size/import-nothing.js',
'tape-compat': 'test/smoke/tape-compat.ts'
}
};

Expand Down
1 change: 1 addition & 0 deletions examples/vite.config.local.mjs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable import/namespace */
import {defineConfig} from 'vite';
import {getOcularConfig} from '@vis.gl/dev-tools';
import {join} from 'path';
Expand Down
13 changes: 12 additions & 1 deletion modules/test-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@
"types": "./dist/index.d.ts",
"import": "./dist/index.js",
"require": "./dist/index.cjs"
},
"./vitest": {
"types": "./dist/vitest.d.ts",
"import": "./dist/vitest.js",
"require": "./dist/vitest.cjs"
}
},
"files": [
Expand All @@ -39,7 +44,13 @@
"@deck.gl/core": "~9.2.0",
"@luma.gl/core": "~9.3.0-alpha.2",
"@luma.gl/engine": "~9.3.0-alpha.2",
"@probe.gl/test-utils": "^4.1.1"
"@probe.gl/test-utils": "^4.1.1",
"vitest": "^4.0.18"
},
"peerDependenciesMeta": {
"vitest": {
"optional": true
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probe.gl peer dependency not optional for vitest users

Low Severity

@probe.gl/test-utils remains a required (non-optional) peer dependency, but the new ./vitest entry point is specifically designed to avoid importing probe.gl. Vitest-only users who don't need probe.gl will still receive peer dependency warnings from their package manager. @probe.gl/test-utils needs to be added to peerDependenciesMeta as optional, similar to how vitest is configured.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is intentional since that could be a breaking change to tape users on the next minor release

},
"gitHead": "13ace64fc2cee08c133afc882fc307253489a4e4"
}
42 changes: 42 additions & 0 deletions modules/test-utils/src/globals.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// deck.gl
// SPDX-License-Identifier: MIT
// Copyright (c) vis.gl contributors

// Type declarations for browser test driver functions injected by @probe.gl/test-utils

interface BrowserTestDriverDiffOptions {
goldenImage: string;
region?: {x: number; y: number; width: number; height: number};
saveOnFail?: boolean;
saveAs?: string;
threshold?: number;
createDiffImage?: boolean;
tolerance?: number;
includeAA?: boolean;
includeEmpty?: boolean;
platform?: string;
}

interface BrowserTestDriverDiffResult {
headless: boolean;
match: string | number;
matchPercentage: string;
success: boolean;
error: Error | string | null;
}

interface BrowserTestDriverInputEvent {
type: string;
[key: string]: any;
}

declare global {
interface Window {
browserTestDriver_emulateInput(event: BrowserTestDriverInputEvent): Promise<void>;
browserTestDriver_captureAndDiffScreen(
options: BrowserTestDriverDiffOptions
): Promise<BrowserTestDriverDiffResult>;
}
}

export {};
10 changes: 3 additions & 7 deletions modules/test-utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,8 @@ export {toLowPrecision} from './utils/precision';
export {gl, device} from './utils/setup-gl';

// Utilities for update tests (lifecycle tests)
export {
testLayer,
testLayerAsync,
testInitializeLayer,
testInitializeLayerAsync
} from './lifecycle-test';
// Re-export from tape.ts which provides default spy factory for backward compat
export {testLayer, testLayerAsync, testInitializeLayer, testInitializeLayerAsync} from './tape';
export {generateLayerTests} from './generate-layer-tests';

// Basic utility for rendering multiple scenes (could go into "deck.gl/core")
Expand All @@ -23,6 +19,6 @@ export {SnapshotTestRunner} from './snapshot-test-runner';
// A utility that emulates input events
export {InteractionTestRunner} from './interaction-test-runner';

export type {LayerTestCase} from './lifecycle-test';
export type {LayerTestCase, ResetSpy, SpyFactory} from './tape';
export type {SnapshotTestCase} from './snapshot-test-runner';
export type {InteractionTestCase} from './interaction-test-runner';
135 changes: 94 additions & 41 deletions modules/test-utils/src/lifecycle-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

import {LayerManager, MapView, DeckRenderer} from '@deck.gl/core';

import {makeSpy} from '@probe.gl/test-utils';
import {device} from './utils/setup-gl';

import type {Layer, CompositeLayer, Viewport} from '@deck.gl/core';
Expand Down Expand Up @@ -128,8 +127,25 @@ export async function testInitializeLayerAsync(
return null;
}

// TODO - export from probe.gl
type Spy = ReturnType<typeof makeSpy>;
/** Spy object compatible with both vitest and probe.gl */
export type Spy = {
/** Restore the original method (vitest) */
mockRestore?: () => void;
/** Restore the original method (probe.gl) */
restore?: () => void;
/** Call history (vitest) */
mock?: {calls: unknown[][]};
/** Call history (probe.gl) */
calls?: unknown[][];
/** Whether the spy was called (probe.gl) */
called?: boolean;
};

/** Factory function to create a spy on an object method */
export type SpyFactory = (obj: object, method: string) => Spy;

/** Function to reset/cleanup a spy after each test case */
export type ResetSpy = (spy: Spy) => void;

export type LayerClass<LayerT extends Layer> = {
new (...args): LayerT;
Expand Down Expand Up @@ -167,11 +183,7 @@ type TestResources = {
oldResourceCounts: Record<string, number>;
};

/**
* Initialize and updates a layer over a sequence of scenarios (test cases).
* Use `testLayerAsync` if the layer's update flow contains async operations.
*/
export function testLayer<LayerT extends Layer>(opts: {
export type TestLayerOptions<LayerT extends Layer> = {
/** The layer class to test against */
Layer: LayerClass<LayerT>;
/** The initial viewport
Expand All @@ -189,8 +201,18 @@ export function testLayer<LayerT extends Layer>(opts: {
spies?: string[];
/** Callback if any error is thrown */
onError?: (error: Error, title: string) => void;
}): void {
const {Layer, testCases = [], spies = [], onError = defaultOnError} = opts;
/** Factory function to create spies */
createSpy: SpyFactory;
/** Function to reset/cleanup a spy after each test case */
resetSpy: ResetSpy;
};

/**
* Initialize and updates a layer over a sequence of scenarios (test cases).
* Use `testLayerAsync` if the layer's update flow contains async operations.
*/
export function testLayer<LayerT extends Layer>(opts: TestLayerOptions<LayerT>): void {
const {Layer, testCases = [], spies = [], onError = defaultOnError, createSpy, resetSpy} = opts;

const resources = setupLayerTests(`testing ${Layer.layerName}`, opts);

Expand All @@ -200,12 +222,18 @@ export function testLayer<LayerT extends Layer>(opts: {
// Save old state before update
const oldState = {...layer.state};

const {layer: newLayer, spyMap} = runLayerTestUpdate(testCase, resources, layer, spies);
const {layer: newLayer, spyMap} = runLayerTestUpdate(
testCase,
resources,
layer,
spies,
createSpy
);

runLayerTestPostUpdateCheck(testCase, newLayer, oldState, spyMap);

// Remove spies
Object.keys(spyMap).forEach(k => spyMap[k].reset());
// Reset spies between test cases
Object.keys(spyMap).forEach(k => resetSpy(spyMap[k]));
layer = newLayer;
}

Expand All @@ -219,26 +247,10 @@ export function testLayer<LayerT extends Layer>(opts: {
* Initialize and updates a layer over a sequence of scenarios (test cases).
* Each test case is awaited until the layer's isLoaded flag is true.
*/
export async function testLayerAsync<LayerT extends Layer>(opts: {
/** The layer class to test against */
Layer: LayerClass<LayerT>;
/** The initial viewport
* @default WebMercatorViewport
*/
viewport?: Viewport;
/**
* If provided, used to controls time progression. Useful for testing transitions and animations.
*/
timeline?: Timeline;
testCases?: LayerTestCase<LayerT>[];
/**
* List of layer method names to watch
*/
spies?: string[];
/** Callback if any error is thrown */
onError?: (error: Error, title: string) => void;
}): Promise<void> {
const {Layer, testCases = [], spies = [], onError = defaultOnError} = opts;
export async function testLayerAsync<LayerT extends Layer>(
opts: TestLayerOptions<LayerT>
): Promise<void> {
const {Layer, testCases = [], spies = [], onError = defaultOnError, createSpy, resetSpy} = opts;

const resources = setupLayerTests(`testing ${Layer.layerName}`, opts);

Expand All @@ -248,7 +260,13 @@ export async function testLayerAsync<LayerT extends Layer>(opts: {
// Save old state before update
const oldState = {...layer.state};

const {layer: newLayer, spyMap} = runLayerTestUpdate(testCase, resources, layer, spies);
const {layer: newLayer, spyMap} = runLayerTestUpdate(
testCase,
resources,
layer,
spies,
createSpy
);

runLayerTestPostUpdateCheck(testCase, newLayer, oldState, spyMap);

Expand All @@ -257,12 +275,13 @@ export async function testLayerAsync<LayerT extends Layer>(opts: {
runLayerTestPostUpdateCheck(testCase, newLayer, oldState, spyMap);
}

// Remove spies
Object.keys(spyMap).forEach(k => spyMap[k].reset());
// Reset spies between test cases
Object.keys(spyMap).forEach(k => resetSpy(spyMap[k]));
layer = newLayer;
}

const error = cleanupAfterLayerTests(resources);
// Use async cleanup to allow pending luma.gl async operations to complete
const error = await cleanupAfterLayerTestsAsync(resources);
if (error) {
onError(error, `${Layer.layerName} should delete all resources`);
}
Expand Down Expand Up @@ -317,6 +336,39 @@ function cleanupAfterLayerTests({
return null;
}

/**
* Async cleanup that waits for pending async operations before finalizing resources.
* This prevents unhandled rejections from luma.gl's async shader error reporting
* which may try to access destroyed WebGL resources if cleanup happens too early.
*/
async function cleanupAfterLayerTestsAsync({
layerManager,
deckRenderer,
oldResourceCounts
}: TestResources): Promise<Error | null> {
layerManager.setLayers([]);

// Wait for any pending async operations (e.g., luma.gl's deferred shader compilation
// error handling) to complete before destroying resources. This prevents
// "getProgramInfoLog" errors when async error reporting tries to access
// already-destroyed WebGL programs.
await new Promise(resolve => setTimeout(resolve, 0));

layerManager.finalize();
deckRenderer.finalize();

const resourceCounts = getResourceCounts();

for (const resourceName in resourceCounts) {
if (resourceCounts[resourceName] !== oldResourceCounts[resourceName]) {
return new Error(
`${resourceCounts[resourceName] - oldResourceCounts[resourceName]} ${resourceName}s`
);
}
}
return null;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicated cleanup logic across sync and async functions

Low Severity

cleanupAfterLayerTestsAsync duplicates nearly all logic from cleanupAfterLayerTests — the setLayers([]), finalize() calls, and the resource count comparison loop are copy-pasted. The only meaningful difference is the await new Promise(resolve => setTimeout(resolve, 0)) between setLayers and finalize. Extracting the shared resource-checking logic into a helper, or having the async version call the sync one after its await, would reduce the maintenance risk of these diverging over time.

Additional Locations (1)

Fix in Cursor Fix in Web


function getResourceCounts(): Record<string, number> {
/* global luma */
const resourceStats = (luma.stats as StatsManager).get('Resource Counts');
Expand All @@ -326,11 +378,11 @@ function getResourceCounts(): Record<string, number> {
};
}

function injectSpies(layer: Layer, spies: string[]): Record<string, Spy> {
function injectSpies(layer: Layer, spies: string[], spyFactory: SpyFactory): Record<string, Spy> {
const spyMap: Record<string, Spy> = {};
if (spies) {
for (const functionName of spies) {
spyMap[functionName] = makeSpy(Object.getPrototypeOf(layer), functionName);
spyMap[functionName] = spyFactory(Object.getPrototypeOf(layer), functionName);
}
}
return spyMap;
Expand Down Expand Up @@ -366,7 +418,8 @@ function runLayerTestUpdate<LayerT extends Layer>(
testCase: LayerTestCase<LayerT>,
{layerManager, deckRenderer}: TestResources,
layer: LayerT,
spies: string[]
spies: string[],
spyFactory: SpyFactory
): {
layer: LayerT;
spyMap: Record<string, Spy>;
Expand All @@ -387,7 +440,7 @@ function runLayerTestUpdate<LayerT extends Layer>(

// Create a map of spies that the test case can inspect
spies = testCase.spies || spies;
const spyMap = injectSpies(layer, spies);
const spyMap = injectSpies(layer, spies, spyFactory);
const drawLayers = () => {
deckRenderer.renderLayers({
pass: 'test',
Expand Down
Loading
Loading