Skip to content

Commit 8ea4b0b

Browse files
committed
downgrade from error to warn, to be changed in v9
also, learned about mocking warn to not display a message when running the jest logs. should clean up the logs for these tests, and an additional test in Trial.spec.ts.
1 parent 761ae37 commit 8ea4b0b

3 files changed

Lines changed: 46 additions & 28 deletions

File tree

.changeset/eleven-drinks-fail.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"jspsych": patch
33
---
44

5-
parameter types will properly be checked in case of type mismatch, along with `ParameterType.SELECT` params properly using the `option` field to check if it is a valid parameter
5+
parameter types will properly be checked in case of type mismatch, along with `ParameterType.SELECT` params properly using the `option` field to check if it is a valid parameter. this will only warn the developer, but in v9 we plan to make it error.

packages/jspsych/src/timeline/Trial.spec.ts

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ describe("Trial", () => {
235235
});
236236

237237
it("respects the `save_trial_parameters` parameter", async () => {
238-
const consoleSpy = jest.spyOn(console, "warn").mockImplementation();
238+
const consoleSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
239239

240240
TestPlugin.setParameterInfos({
241241
stringParameter1: { type: ParameterType.STRING },
@@ -561,6 +561,17 @@ describe("Trial", () => {
561561
});
562562

563563
describe("with parameter type mismatches", () => {
564+
//TODO: redo these to expect errors on v9!
565+
let consoleSpy: jest.SpyInstance;
566+
567+
beforeEach(() => {
568+
consoleSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
569+
});
570+
571+
afterEach(() => {
572+
consoleSpy.mockRestore();
573+
});
574+
564575
it("errors on non-boolean values for boolean parameters", async () => {
565576
TestPlugin.setParameterInfos({
566577
boolParameter: { type: ParameterType.BOOL },
@@ -570,8 +581,9 @@ describe("Trial", () => {
570581
await createTrial({ type: TestPlugin, boolParameter: true }).run();
571582

572583
// this shouldn't:
573-
await expect(createTrial({ type: TestPlugin, boolParameter: "foo" }).run()).rejects.toThrow(
574-
"A non-boolean value (`foo`) was provided for the boolean parameter \"boolParameter\" in the \"test\" plugin."
584+
await createTrial({ type: TestPlugin, boolParameter: "foo" }).run();
585+
expect(consoleSpy).toHaveBeenCalledWith(
586+
'A non-boolean value (`foo`) was provided for the boolean parameter "boolParameter" in the "test" plugin.'
575587
);
576588
});
577589

@@ -584,8 +596,9 @@ describe("Trial", () => {
584596
await createTrial({ type: TestPlugin, stringParameter: "foo" }).run();
585597

586598
// this shouldn't:
587-
await expect(createTrial({ type: TestPlugin, stringParameter: 1 }).run()).rejects.toThrow(
588-
"A non-string value (`1`) was provided for the parameter \"stringParameter\" in the \"test\" plugin."
599+
await createTrial({ type: TestPlugin, stringParameter: 1 }).run();
600+
expect(consoleSpy).toHaveBeenCalledWith(
601+
'A non-string value (`1`) was provided for the parameter "stringParameter" in the "test" plugin.'
589602
);
590603
});
591604

@@ -599,15 +612,17 @@ describe("Trial", () => {
599612
await createTrial({ type: TestPlugin, intParameter: 1, floatParameter: 1.5 }).run();
600613

601614
// this shouldn't:
602-
await expect(createTrial({ type: TestPlugin, intParameter: "foo", floatParameter: 1.5 }).run()).rejects.toThrow(
603-
"A non-numeric value (`foo`) was provided for the numeric parameter \"intParameter\" in the \"test\" plugin."
615+
await createTrial({ type: TestPlugin, intParameter: "foo", floatParameter: 1.5 }).run();
616+
expect(consoleSpy).toHaveBeenCalledWith(
617+
'A non-numeric value (`foo`) was provided for the numeric parameter "intParameter" in the "test" plugin.'
604618
);
605-
await expect(createTrial({ type: TestPlugin, intParameter: 1, floatParameter: "foo" }).run()).rejects.toThrow(
606-
"A non-numeric value (`foo`) was provided for the numeric parameter \"floatParameter\" in the \"test\" plugin."
619+
620+
await createTrial({ type: TestPlugin, intParameter: 1, floatParameter: "foo" }).run();
621+
expect(consoleSpy).toHaveBeenCalledWith(
622+
'A non-numeric value (`foo`) was provided for the numeric parameter "floatParameter" in the "test" plugin.'
607623
);
608624

609-
// this should warn but not error:
610-
const consoleSpy = jest.spyOn(console, "warn").mockImplementation();
625+
// this should warn but not error (behavior in v9):
611626
await createTrial({ type: TestPlugin, intParameter: 1.5, floatParameter: 1.5 }).run();
612627
expect(consoleSpy).toHaveBeenCalledWith(
613628
`A float value (\`1.5\`) was provided for the integer parameter "intParameter" in the "test" plugin. The value will be truncated to an integer.`
@@ -623,12 +638,13 @@ describe("Trial", () => {
623638
await createTrial({ type: TestPlugin, functionParameter: () => {} }).run();
624639

625640
// this shouldn't:
626-
await expect(createTrial({ type: TestPlugin, functionParameter: "foo" }).run()).rejects.toThrow(
627-
"A non-function value (`foo`) was provided for the function parameter \"functionParameter\" in the \"test\" plugin."
641+
await createTrial({ type: TestPlugin, functionParameter: "foo" }).run();
642+
expect(consoleSpy).toHaveBeenCalledWith(
643+
'A non-function value (`foo`) was provided for the function parameter "functionParameter" in the "test" plugin.'
628644
);
629645
});
630646

631-
it("errors on select parameters with values not in the options", async () => {
647+
it("errors on select parameters with values not in the options array", async () => {
632648
TestPlugin.setParameterInfos({
633649
selectParameter: {
634650
type: ParameterType.SELECT,
@@ -640,8 +656,10 @@ describe("Trial", () => {
640656
await createTrial({ type: TestPlugin, selectParameter: "foo" }).run();
641657

642658
// this shouldn't:
643-
await expect(createTrial({ type: TestPlugin, selectParameter: "baz" }).run()).rejects.toThrow(
644-
"The value \"baz\" is not a valid option for the parameter \"selectParameter\" in the \"test\" plugin. Valid options are: foo, bar."
659+
660+
await createTrial({ type: TestPlugin, selectParameter: "baz" }).run();
661+
expect(consoleSpy).toHaveBeenCalledWith(
662+
'The value "baz" is not a valid option for the parameter "selectParameter" in the "test" plugin. Valid options are: foo, bar.'
645663
);
646664
});
647665
});

packages/jspsych/src/timeline/Trial.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -380,21 +380,21 @@ export class Trial extends TimelineNode {
380380
},
381381
});
382382

383+
// TODO: ensure that this throws an error in v9!
383384
// major parameter type validation
384385
if (!parameterConfig.array && parameterValue !== null) {
385386
switch (parameterConfig.type) {
386387
case ParameterType.BOOL:
387388
if (typeof parameterValue !== "boolean") {
388389
const parameterPathString = parameterPathArrayToString(parameterPath);
389-
throw new Error(
390+
console.warn(
390391
`A non-boolean value (\`${parameterValue}\`) was provided for the boolean parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
391392
);
392393
}
393394
break;
394395
// @ts-ignore falls through
395396
case ParameterType.KEYS: // "ALL_KEYS", "NO_KEYS", and single key strings are checked here
396-
if (Array.isArray(parameterValue))
397-
break;
397+
if (Array.isArray(parameterValue)) break;
398398
case ParameterType.STRING:
399399
case ParameterType.HTML_STRING:
400400
case ParameterType.KEY:
@@ -403,7 +403,7 @@ export class Trial extends TimelineNode {
403403
case ParameterType.IMAGE:
404404
if (typeof parameterValue !== "string") {
405405
const parameterPathString = parameterPathArrayToString(parameterPath);
406-
throw new Error(
406+
console.warn(
407407
`A non-string value (\`${parameterValue}\`) was provided for the parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
408408
);
409409
}
@@ -412,23 +412,23 @@ export class Trial extends TimelineNode {
412412
case ParameterType.INT:
413413
if (typeof parameterValue !== "number") {
414414
const parameterPathString = parameterPathArrayToString(parameterPath);
415-
throw new Error(
415+
console.warn(
416416
`A non-numeric value (\`${parameterValue}\`) was provided for the numeric parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
417417
);
418418
}
419419
break;
420420
case ParameterType.FUNCTION:
421421
if (typeof parameterValue !== "function") {
422422
const parameterPathString = parameterPathArrayToString(parameterPath);
423-
throw new Error(
423+
console.warn(
424424
`A non-function value (\`${parameterValue}\`) was provided for the function parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
425425
);
426426
}
427427
break;
428428
case ParameterType.SELECT:
429429
if (!parameterConfig.options) {
430430
const parameterPathString = parameterPathArrayToString(parameterPath);
431-
throw new Error(
431+
console.warn(
432432
`The "options" array is required for the "select" parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin.`
433433
);
434434
}
@@ -448,10 +448,10 @@ export class Trial extends TimelineNode {
448448
if (parameterConfig.type === ParameterType.SELECT) {
449449
if (!parameterConfig.options.includes(parameterValue)) {
450450
const parameterPathString = parameterPathArrayToString(parameterPath);
451-
throw new Error(
452-
`The value "${parameterValue}" is not a valid option for the parameter "${parameterPathString}" in the "${this.pluginInfo.name}" plugin. Valid options are: ${parameterConfig.options.join(
453-
", "
454-
)}.`
451+
console.warn(
452+
`The value "${parameterValue}" is not a valid option for the parameter "${parameterPathString}" in the "${
453+
this.pluginInfo.name
454+
}" plugin. Valid options are: ${parameterConfig.options.join(", ")}.`
455455
);
456456
}
457457
}

0 commit comments

Comments
 (0)