Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make test params readonly so they can't be accidentally permanently modified #3097

Merged
merged 7 commits into from
Oct 26, 2023
Merged
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
16 changes: 10 additions & 6 deletions src/common/framework/params_builder.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Merged, mergeParams, mergeParamsChecked } from '../internal/params_utils.js';
import { comparePublicParamsPaths, Ordering } from '../internal/query/compare.js';
import { stringifyPublicParams } from '../internal/query/stringify_params.js';
import { DeepReadonly } from '../util/types.js';
import { assert, mapLazy, objectEquals } from '../util/util.js';

import { TestParams } from './fixture.js';
Expand Down Expand Up @@ -98,7 +99,7 @@ export type ParamTypeOf<
* - `[case params, undefined]` if not.
*/
export type CaseSubcaseIterable<CaseP, SubcaseP> = Iterable<
readonly [CaseP, Iterable<SubcaseP> | undefined]
readonly [DeepReadonly<CaseP>, Iterable<DeepReadonly<SubcaseP>> | undefined]
>;

/**
Expand Down Expand Up @@ -143,7 +144,7 @@ export function builderIterateCasesWithSubcases(
*/
export class CaseParamsBuilder<CaseP extends {}>
extends ParamsBuilderBase<CaseP, {}>
implements Iterable<CaseP>, ParamsBuilder {
implements Iterable<DeepReadonly<CaseP>>, ParamsBuilder {
*iterateCasesWithSubcases(caseFilter: TestParams | null): CaseSubcaseIterable<CaseP, {}> {
for (const caseP of this.cases(caseFilter)) {
if (caseFilter) {
Expand All @@ -155,12 +156,12 @@ export class CaseParamsBuilder<CaseP extends {}>
}
}

yield [caseP, undefined];
yield [caseP as DeepReadonly<typeof caseP>, undefined];
}
}

[Symbol.iterator](): Iterator<CaseP> {
return this.cases(null);
[Symbol.iterator](): Iterator<DeepReadonly<CaseP>> {
return this.cases(null) as Iterator<DeepReadonly<CaseP>>;
}

/** @inheritDoc */
Expand Down Expand Up @@ -302,7 +303,10 @@ export class SubcaseParamsBuilder<CaseP extends {}, SubcaseP extends {}>

const subcases = Array.from(this.subcases(caseP));
if (subcases.length) {
yield [caseP, subcases];
yield [
caseP as DeepReadonly<typeof caseP>,
subcases as DeepReadonly<typeof subcases[number]>[],
];
}
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/common/internal/test_group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
stringifyPublicParamsUniquely,
} from '../internal/query/stringify_params.js';
import { validQueryPart } from '../internal/query/validQueryPart.js';
import { DeepReadonly } from '../util/types.js';
import { assert, unreachable } from '../util/util.js';

import { logToWebsocket } from './websocket_logger.js';
Expand Down Expand Up @@ -82,9 +83,11 @@ export function makeTestGroupForUnitTesting<F extends Fixture>(
/** Parameter name for batch number (see also TestBuilder.batch). */
const kBatchParamName = 'batch__';

type TestFn<F extends Fixture, P extends {}> = (t: F & { params: P }) => Promise<void> | void;
type TestFn<F extends Fixture, P extends {}> = (
t: F & { params: DeepReadonly<P> }
) => Promise<void> | void;
type BeforeAllSubcasesFn<S extends SubcaseBatchState, P extends {}> = (
s: S & { params: P }
s: S & { params: DeepReadonly<P> }
) => Promise<void> | void;

export class TestGroup<F extends Fixture> implements TestGroupBuilder<F> {
Expand Down
39 changes: 39 additions & 0 deletions src/common/util/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,45 @@ export type TypeEqual<X, Y> = (<T>() => T extends X ? 1 : 2) extends <T>() => T
/* eslint-disable-next-line @typescript-eslint/no-unused-vars */
export function assertTypeTrue<T extends true>() {}

/** `ReadonlyArray` of `ReadonlyArray`s. */
export type ROArrayArray<T> = ReadonlyArray<ReadonlyArray<T>>;
/** `ReadonlyArray` of `ReadonlyArray`s of `ReadonlyArray`s. */
export type ROArrayArrayArray<T> = ReadonlyArray<ReadonlyArray<ReadonlyArray<T>>>;

/**
* Deep version of the Readonly<> type, with support for tuples (up to length 7).
* <https://gist.github.com/masterkidan/7322752f569b1bba53e0426266768623>
*/
export type DeepReadonly<T> = T extends [infer A]
? DeepReadonlyObject<[A]>
: T extends [infer A, infer B]
? DeepReadonlyObject<[A, B]>
: T extends [infer A, infer B, infer C]
? DeepReadonlyObject<[A, B, C]>
: T extends [infer A, infer B, infer C, infer D]
? DeepReadonlyObject<[A, B, C, D]>
: T extends [infer A, infer B, infer C, infer D, infer E]
? DeepReadonlyObject<[A, B, C, D, E]>
: T extends [infer A, infer B, infer C, infer D, infer E, infer F]
? DeepReadonlyObject<[A, B, C, D, E, F]>
: T extends [infer A, infer B, infer C, infer D, infer E, infer F, infer G]
? DeepReadonlyObject<[A, B, C, D, E, F, G]>
: T extends Map<infer U, infer V>
? ReadonlyMap<DeepReadonlyObject<U>, DeepReadonlyObject<V>>
: T extends Set<infer U>
? ReadonlySet<DeepReadonlyObject<U>>
: T extends Promise<infer U>
? Promise<DeepReadonlyObject<U>>
: T extends Primitive
? T
: T extends (infer A)[]
? DeepReadonlyArray<A>
: DeepReadonlyObject<T>;

type Primitive = string | number | boolean | undefined | null | Function | symbol;
type DeepReadonlyArray<T> = ReadonlyArray<DeepReadonly<T>>;
type DeepReadonlyObject<T> = { readonly [P in keyof T]: DeepReadonly<T[P]> };

/**
* Computes the intersection of a set of types, given the union of those types.
*
Expand Down
4 changes: 2 additions & 2 deletions src/common/util/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ interface TypedArrayMap {

type TypedArrayParam<K extends keyof TypedArrayMap> = {
type: K;
data: number[];
data: readonly number[];
};

/**
Expand Down Expand Up @@ -387,7 +387,7 @@ export function typedArrayParam<K extends keyof TypedArrayMap>(

export function createTypedArray<K extends keyof TypedArrayMap>(
type: K,
data: number[]
data: readonly number[]
): TypedArrayMap[K] {
return new kTypedArrayBufferViews[type](data) as TypedArrayMap[K];
}
Expand Down
30 changes: 10 additions & 20 deletions src/unittests/floating_point.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2582,8 +2582,7 @@ g.test('atanInterval')
return ulp_error * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.atanInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -2760,8 +2759,7 @@ g.test('cosInterval')
return t.params.trait === 'f32' ? 2 ** -11 : 2 ** -7;
};

t.params.expected = applyError(t.params.expected, error);
lokokung marked this conversation as resolved.
Show resolved Hide resolved
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.cosInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -2941,8 +2939,7 @@ g.test('expInterval')
return ulp_error * trait.oneULP(x);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));
const got = trait.expInterval(t.params.input);

t.expect(
Expand Down Expand Up @@ -3001,8 +2998,7 @@ g.test('exp2Interval')
return ulp_error * trait.oneULP(x);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.exp2Interval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3197,8 +3193,7 @@ g.test('inverseSqrtInterval')
return 2 * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.inverseSqrtInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3322,8 +3317,7 @@ g.test('logInterval')
return 3 * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.logInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3373,8 +3367,7 @@ g.test('log2Interval')
return 3 * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.log2Interval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3720,8 +3713,7 @@ g.test('sinInterval')
return t.params.trait === 'f32' ? 2 ** -11 : 2 ** -7;
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.sinInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -3855,8 +3847,7 @@ g.test('sqrtInterval')
return 2.5 * trait.oneULP(n);
};

t.params.expected = applyError(t.params.expected, error);
const expected = trait.toInterval(t.params.expected);
const expected = trait.toInterval(applyError(t.params.expected, error));

const got = trait.sqrtInterval(t.params.input);
t.expect(
Expand Down Expand Up @@ -4429,10 +4420,9 @@ g.test('divisionInterval')
};

const [x, y] = t.params.input;
t.params.expected = applyError(t.params.expected, error);

// Do not swizzle here, so the correct implementation under test is called.
const expected = FP[t.params.trait].toInterval(t.params.expected);
const expected = FP[t.params.trait].toInterval(applyError(t.params.expected, error));
const got = FP[t.params.trait].divisionInterval(x, y);
t.expect(
objectEquals(expected, got),
Expand Down
4 changes: 2 additions & 2 deletions src/unittests/maths.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ function withinOneULPF32(got: number, expected: number, mode: FlushMode): boolea
* FTZ occur during comparison
**/
function compareArrayOfNumbersF32(
got: Array<number>,
expect: Array<number>,
got: readonly number[],
expect: readonly number[],
mode: FlushMode = 'flush'
): boolean {
return (
Expand Down
4 changes: 2 additions & 2 deletions src/webgpu/api/validation/render_pipeline/inter_stage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ function getVarName(i: number) {
}

class InterStageMatchingValidationTest extends CreateRenderPipelineValidationTest {
getVertexStateWithOutputs(outputs: string[]): GPUVertexState {
getVertexStateWithOutputs(outputs: readonly string[]): GPUVertexState {
return {
module: this.device.createShaderModule({
code: `
Expand All @@ -32,7 +32,7 @@ class InterStageMatchingValidationTest extends CreateRenderPipelineValidationTes
}

getFragmentStateWithInputs(
inputs: string[],
inputs: readonly string[],
hasBuiltinPosition: boolean = false
): GPUFragmentState {
return {
Expand Down
2 changes: 1 addition & 1 deletion src/webgpu/format_info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1255,7 +1255,7 @@ export const kFeaturesForFormats = getFeaturesForFormats(kTextureFormats);
/**
* Given an array of texture formats return the number of bytes per sample.
*/
export function computeBytesPerSampleFromFormats(formats: GPUTextureFormat[]) {
export function computeBytesPerSampleFromFormats(formats: readonly GPUTextureFormat[]) {
let bytesPerSample = 0;
for (const format of formats) {
const info = kTextureFormatInfo[format];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const additionVectorScalarInterval = (v: number[], s: number): FPVector => {
const additionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.additionInterval(e, s)));
};

const additionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const additionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.additionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const divisionVectorScalarInterval = (v: number[], s: number): FPVector => {
const divisionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.divisionInterval(e, s)));
};

const divisionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const divisionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.divisionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const multiplicationVectorScalarInterval = (v: number[], s: number): FPVector => {
const multiplicationVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.multiplicationInterval(e, s)));
};

const multiplicationScalarVectorInterval = (s: number, v: number[]): FPVector => {
const multiplicationScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.multiplicationInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const remainderVectorScalarInterval = (v: number[], s: number): FPVector => {
const remainderVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.remainderInterval(e, s)));
};

const remainderScalarVectorInterval = (s: number, v: number[]): FPVector => {
const remainderScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.remainderInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { onlyConstInputSource, run } from '../expression.js';

import { abstractBinary } from './binary.js';

const subtractionVectorScalarInterval = (v: number[], s: number): FPVector => {
const subtractionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.subtractionInterval(e, s)));
};

const subtractionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const subtractionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.abstract.toVector(v.map(e => FP.abstract.subtractionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { allInputSources, run } from '../expression.js';

import { binary, compoundBinary } from './binary.js';

const additionVectorScalarInterval = (v: number[], s: number): FPVector => {
const additionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.additionInterval(e, s)));
};

const additionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const additionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.additionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { allInputSources, run } from '../expression.js';

import { binary, compoundBinary } from './binary.js';

const divisionVectorScalarInterval = (v: number[], s: number): FPVector => {
const divisionVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.divisionInterval(e, s)));
};

const divisionScalarVectorInterval = (s: number, v: number[]): FPVector => {
const divisionScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.divisionInterval(s, e)));
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import { allInputSources, run } from '../expression.js';

import { binary, compoundBinary } from './binary.js';

const multiplicationVectorScalarInterval = (v: number[], s: number): FPVector => {
const multiplicationVectorScalarInterval = (v: readonly number[], s: number): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.multiplicationInterval(e, s)));
};

const multiplicationScalarVectorInterval = (s: number, v: number[]): FPVector => {
const multiplicationScalarVectorInterval = (s: number, v: readonly number[]): FPVector => {
return FP.f16.toVector(v.map(e => FP.f16.multiplicationInterval(s, e)));
};

Expand Down
Loading