Skip to content

Commit

Permalink
Test that DOMExceptions from WebGPU always have stacks (#3105)
Browse files Browse the repository at this point in the history
  • Loading branch information
kainino0x authored Oct 31, 2023
1 parent 3dbe4ce commit ef5d229
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 33 deletions.
31 changes: 25 additions & 6 deletions src/common/framework/fixture.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { TestCaseRecorder } from '../internal/logging/test_case_recorder.js';
import { JSONWithUndefined } from '../internal/params_utils.js';
import { assert, unreachable } from '../util/util.js';
import { assert, ExceptionCheckOptions, unreachable } from '../util/util.js';

export class SkipTestCase extends Error {}
export class UnexpectedPassError extends Error {}
Expand Down Expand Up @@ -237,16 +237,26 @@ export class Fixture<S extends SubcaseBatchState = SubcaseBatchState> {
}

/** Expect that the provided promise rejects, with the provided exception name. */
shouldReject(expectedName: string, p: Promise<unknown>, msg?: string): void {
shouldReject(
expectedName: string,
p: Promise<unknown>,
{ allowMissingStack = false, message }: ExceptionCheckOptions = {}
): void {
this.eventualAsyncExpectation(async niceStack => {
const m = msg ? ': ' + msg : '';
const m = message ? ': ' + message : '';
try {
await p;
niceStack.message = 'DID NOT REJECT' + m;
this.rec.expectationFailed(niceStack);
} catch (ex) {
niceStack.message = 'rejected as expected' + m;
this.expectErrorValue(expectedName, ex, niceStack);
if (!allowMissingStack) {
if (!(ex instanceof Error && typeof ex.stack === 'string')) {
const exMessage = ex instanceof Error ? ex.message : '?';
niceStack.message = `rejected as expected, but missing stack (${exMessage})${m}`;
this.rec.expectationFailed(niceStack);
}
}
}
});
}
Expand All @@ -257,8 +267,12 @@ export class Fixture<S extends SubcaseBatchState = SubcaseBatchState> {
*
* MAINTENANCE_TODO: Change to `string | false` so the exception name is always checked.
*/
shouldThrow(expectedError: string | boolean, fn: () => void, msg?: string): void {
const m = msg ? ': ' + msg : '';
shouldThrow(
expectedError: string | boolean,
fn: () => void,
{ allowMissingStack = false, message }: ExceptionCheckOptions = {}
) {
const m = message ? ': ' + message : '';
try {
fn();
if (expectedError === false) {
Expand All @@ -271,6 +285,11 @@ export class Fixture<S extends SubcaseBatchState = SubcaseBatchState> {
this.rec.expectationFailed(new Error('threw unexpectedly' + m));
} else {
this.expectErrorValue(expectedError, ex, new Error(m));
if (!allowMissingStack) {
if (!(ex instanceof Error && typeof ex.stack === 'string')) {
this.rec.expectationFailed(new Error('threw as expected, but missing stack' + m));
}
}
}
}
}
Expand Down
20 changes: 17 additions & 3 deletions src/common/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,29 @@ export function assertOK<T>(value: Error | T): T {
return value;
}

/** Options for assertReject, shouldReject, and friends. */
export type ExceptionCheckOptions = { allowMissingStack?: boolean; message?: string };

/**
* Resolves if the provided promise rejects; rejects if it does not.
*/
export async function assertReject(p: Promise<unknown>, msg?: string): Promise<void> {
export async function assertReject(
expectedName: string,
p: Promise<unknown>,
{ allowMissingStack = false, message }: ExceptionCheckOptions = {}
): Promise<void> {
try {
await p;
unreachable(msg);
unreachable(message);
} catch (ex) {
// Assertion OK
// Asserted as expected
if (!allowMissingStack) {
const m = message ? ` (${message})` : '';
assert(
ex instanceof Error && typeof ex.stack === 'string',
'threw as expected, but missing stack' + m
);
}
}
}

Expand Down
5 changes: 4 additions & 1 deletion src/unittests/loaders_and_trees.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -703,7 +703,10 @@ async function testIterateCollapsed(
subqueriesToExpand: expectations,
});
if (expectedResult === 'throws') {
t.shouldReject('Error', treePromise, 'loadTree should have thrown Error');
t.shouldReject('Error', treePromise, {
// Some errors here use StacklessError to print nicer command line outputs.
allowMissingStack: true,
});
return;
}
const tree = await treePromise;
Expand Down
2 changes: 1 addition & 1 deletion src/unittests/test_group.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ g.test('invalid_test_name').fn(t => {
() => {
g.test(name).fn(() => {});
},
name
{ message: name }
);
}
});
Expand Down
2 changes: 2 additions & 0 deletions src/webgpu/api/operation/adapter/requestDevice.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ g.test('stale')
// Cause a type error by requesting with an unknown feature.
if (awaitInitialError) {
await assertReject(
'TypeError',
adapter.requestDevice({ requiredFeatures: ['unknown-feature' as GPUFeatureName] })
);
} else {
Expand All @@ -131,6 +132,7 @@ g.test('stale')
// Cause an operation error by requesting with an alignment limit that is not a power of 2.
if (awaitInitialError) {
await assertReject(
'OperationError',
adapter.requestDevice({ requiredLimits: { minUniformBufferOffsetAlignment: 255 } })
);
} else {
Expand Down
1 change: 1 addition & 0 deletions src/webgpu/api/validation/buffer/mapping.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ class F extends ValidationTest {
assert(expectation.rejectName === null, 'mapAsync unexpectedly passed');
} catch (ex) {
assert(ex instanceof Error, 'mapAsync rejected with non-error');
assert(typeof ex.stack === 'string', 'mapAsync rejected without a stack');
assert(expectation.rejectName === ex.name, `mapAsync rejected unexpectedly with: ${ex}`);
assert(
expectation.earlyRejection === rejectedEarly,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ g.test('color_target_state')
)
.params(u =>
u
.combine('isAsync', [false, true])
.combine('format', kOptionalTextureFormats)
.filter(t => !!kTextureFormatInfo[t.format].colorRender)
.combine('enable_required_feature', [true, false])
Expand All @@ -287,10 +288,12 @@ g.test('color_target_state')
}
})
.fn(t => {
const { format, enable_required_feature } = t.params;
const { isAsync, format, enable_required_feature } = t.params;

t.shouldThrow(enable_required_feature ? false : 'TypeError', () => {
t.device.createRenderPipeline({
t.doCreateRenderPipelineTest(
isAsync,
enable_required_feature,
{
layout: 'auto',
vertex: {
module: t.device.createShaderModule({
Expand All @@ -313,8 +316,9 @@ g.test('color_target_state')
entryPoint: 'main',
targets: [{ format }],
},
});
});
},
'TypeError'
);
});

g.test('depth_stencil_state')
Expand All @@ -326,6 +330,7 @@ g.test('depth_stencil_state')
)
.params(u =>
u
.combine('isAsync', [false, true])
.combine('format', kOptionalTextureFormats)
.filter(t => !!(kTextureFormatInfo[t.format].depth || kTextureFormatInfo[t.format].stencil))
.combine('enable_required_feature', [true, false])
Expand All @@ -339,10 +344,12 @@ g.test('depth_stencil_state')
}
})
.fn(t => {
const { format, enable_required_feature } = t.params;
const { isAsync, format, enable_required_feature } = t.params;

t.shouldThrow(enable_required_feature ? false : 'TypeError', () => {
t.device.createRenderPipeline({
t.doCreateRenderPipelineTest(
isAsync,
enable_required_feature,
{
layout: 'auto',
vertex: {
module: t.device.createShaderModule({
Expand Down Expand Up @@ -370,8 +377,9 @@ g.test('depth_stencil_state')
entryPoint: 'main',
targets: [{ format: 'rgba8unorm' }],
},
});
});
},
'TypeError'
);
});

g.test('render_bundle_encoder_descriptor_color_format')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,9 @@ export class LimitTestsImpl extends GPUTestBase {
requiredFeatures?: GPUFeatureName[]
) {
if (shouldReject) {
this.shouldReject('OperationError', adapter.requestDevice({ requiredLimits }));
this.shouldReject('OperationError', adapter.requestDevice({ requiredLimits }), {
allowMissingStack: true,
});
return undefined;
} else {
return await adapter.requestDevice({ requiredLimits, requiredFeatures });
Expand Down Expand Up @@ -562,12 +564,12 @@ export class LimitTestsImpl extends GPUTestBase {
expectedName: string,
p: Promise<unknown>,
shouldReject: boolean,
msg?: string
message?: string
): Promise<void> {
if (shouldReject) {
this.shouldReject(expectedName, p, msg);
this.shouldReject(expectedName, p, { message });
} else {
this.shouldResolve(p, msg);
this.shouldResolve(p, message);
}

// We need to explicitly wait for the promise because the device may be
Expand Down
8 changes: 4 additions & 4 deletions src/webgpu/examples.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ g.test('basic').fn(t => {
throw new TypeError();
},
// Log message.
'function should throw Error'
{ message: 'function should throw Error' }
);
});

Expand All @@ -59,17 +59,17 @@ g.test('basic,async').fn(t => {
// Promise expected to reject.
Promise.reject(new TypeError()),
// Log message.
'Promise.reject should reject'
{ message: 'Promise.reject should reject' }
);

// Promise can also be an IIFE.
// Promise can also be an IIFE (immediately-invoked function expression).
t.shouldReject(
'TypeError',
// eslint-disable-next-line @typescript-eslint/require-await
(async () => {
throw new TypeError();
})(),
'Promise.reject should reject'
{ message: 'Promise.reject should reject' }
);
});

Expand Down
8 changes: 4 additions & 4 deletions src/webgpu/util/device_pool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -378,10 +378,10 @@ class DeviceHolder implements DeviceProvider {
await this.device.queue.onSubmittedWorkDone();
}

await assertReject(
this.device.popErrorScope(),
'There was an extra error scope on the stack after a test'
);
await assertReject('OperationError', this.device.popErrorScope(), {
allowMissingStack: true,
message: 'There was an extra error scope on the stack after a test',
});

if (gpuOutOfMemoryError !== null) {
assert(gpuOutOfMemoryError instanceof GPUOutOfMemoryError);
Expand Down

0 comments on commit ef5d229

Please sign in to comment.