Skip to content

Commit

Permalink
Add and pass regression tests for PerseusItem parser (#1907)
Browse files Browse the repository at this point in the history
Issue: LEMS-2582

## Test plan:

`yarn test`

Author: benchristel

Reviewers: jeremywiebe, benchristel, anakaren-rojas, catandthemachines, nishasy

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ gerald

Pull Request URL: #1907
  • Loading branch information
benchristel authored Nov 26, 2024
1 parent 841551a commit 3dbca96
Show file tree
Hide file tree
Showing 28 changed files with 3,148 additions and 75 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilled-turtles-drive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

Internal: add and pass regression tests for `PerseusItem` parser.
19 changes: 12 additions & 7 deletions packages/perseus/src/perseus-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,14 @@ export type PerseusItem = {
hints: ReadonlyArray<Hint>;
// Details about the tools the user might need to answer the question
answerArea: PerseusAnswerArea | null | undefined;
// The version of the item. Not used by Perseus
itemDataVersion: Version;
// Deprecated field
/**
* The version of the item.
* @deprecated Not used.
*/
itemDataVersion: any;
/**
* @deprecated Superseded by per-widget answers.
*/
answer: any;
};

Expand Down Expand Up @@ -1034,17 +1039,17 @@ export type PerseusMatcherWidgetOptions = {
export type PerseusMatrixWidgetAnswers = ReadonlyArray<ReadonlyArray<number>>;
export type PerseusMatrixWidgetOptions = {
// Translatable Text; Shown before the matrix
prefix: string;
prefix?: string | undefined;
// Translatable Text; Shown after the matrix
suffix: string;
suffix?: string | undefined;
// A data matrix representing the "correct" answers to be entered into the matrix
answers: PerseusMatrixWidgetAnswers;
// The coordinate location of the cursor position at start. default: [0, 0]
cursorPosition: ReadonlyArray<number>;
cursorPosition?: ReadonlyArray<number> | undefined;
// The coordinate size of the matrix. Only supports 2-dimensional matrix. default: [3, 3]
matrixBoardSize: ReadonlyArray<number>;
// Whether this is meant to statically display the answers (true) or be used as an input field, graded against the answers
static: boolean;
static?: boolean | undefined;
};

export type PerseusMeasurerWidgetOptions = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {success} from "../result";

import type {Parser} from "../parser-types";

export function defaulted<T, Default extends T | null | undefined>(
export function defaulted<T, Default>(
parser: Parser<T>,
fallback: (missingValue: null | undefined) => Default,
): Parser<T | Default> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ describe("parseAndTypecheckPerseusItem", () => {
});

it("returns an error given an invalid PerseusItem", () => {
const result = parseAndTypecheckPerseusItem(`{ "bad": "value" }`);
const result = parseAndTypecheckPerseusItem(
`{"question": "bad value"}`,
);

assertFailure(result);
expect(result.detail).toContain(
"At (root).question -- expected object, but got undefined",
`At (root).question -- expected object, but got "bad value"`,
);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
optional,
string,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseWidget} from "./widget";

Expand All @@ -19,7 +20,7 @@ export const parseCategorizerWidget: Parser<CategorizerWidget> = parseWidget(
items: array(string),
categories: array(string),
randomizeItems: boolean,
static: boolean,
static: defaulted(boolean, () => false),
values: array(number),
highlightLint: optional(boolean),
linterContext: optional(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
object,
string,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseWidget} from "./widget";

Expand All @@ -15,7 +16,7 @@ export const parseDropdownWidget: Parser<DropdownWidget> = parseWidget(
constant("dropdown"),
object({
placeholder: string,
static: boolean,
static: defaulted(boolean, () => false),
choices: array(
object({
content: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,12 @@ import {
boolean,
constant,
enumeration,
number,
object,
optional,
pipeParsers,
string,
union,
} from "../general-purpose-parsers";

import {parseWidget} from "./widget";
Expand All @@ -21,7 +24,9 @@ const parseAnswerForm: Parser<PerseusExpressionAnswerForm> = object({
form: boolean,
simplify: boolean,
considered: enumeration("correct", "wrong", "ungraded"),
key: optional(string),
key: pipeParsers(optional(union(string).or(number).parser)).then(
(key, ctx) => ctx.success(String(key)),
).parser,
});

export const parseExpressionWidget: Parser<ExpressionWidget> = parseWidget(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
record,
string,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parsePerseusRenderer} from "./perseus-renderer";
import {parseWidget} from "./widget";
Expand All @@ -17,7 +18,7 @@ import type {GradedGroupWidget} from "../../../perseus-types";
import type {Parser} from "../parser-types";

export const parseGradedGroupWidgetOptions = object({
title: string,
title: defaulted(string, () => ""),
hasHint: optional(nullable(boolean)),
// This module has an import cycle with parsePerseusRenderer.
// The anonymous function below ensures that we don't try to access
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import {
array,
boolean,
number,
object,
optional,
record,
string,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseImages} from "./images-map";
import {parseWidgetsMap} from "./widgets-map";

import type {Hint} from "../../../perseus-types";
Expand All @@ -16,13 +16,7 @@ import type {Parser} from "../parser-types";
export const parseHint: Parser<Hint> = object({
replace: optional(boolean),
content: string,
widgets: parseWidgetsMap,
widgets: defaulted(parseWidgetsMap, () => ({})),
metadata: optional(array(string)),
images: record(
string,
object({
width: number,
height: number,
}),
),
images: parseImages,
});
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
string,
union,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseWidget} from "./widget";

Expand All @@ -23,6 +24,6 @@ export const parseIframeWidget: Parser<IFrameWidget> = parseWidget(
height: union(number).or(string).parser,
allowFullScreen: boolean,
allowTopNavigation: optional(boolean),
static: boolean,
static: defaulted(boolean, () => false),
}),
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import {number, object, record, string} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import type {PerseusImageDetail} from "../../../perseus-types";
import type {Parser} from "../parser-types";

export const parseImages: Parser<{[key: string]: PerseusImageDetail}> =
defaulted(
record(
string,
object({
width: number,
height: number,
}),
),
() => ({}),
);
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,24 @@ import {
constant,
number,
object,
optional,
string,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseWidget} from "./widget";

import type {MatrixWidget} from "../../../perseus-types";
import type {Parser} from "../parser-types";

export const parseMatrixWidget: Parser<MatrixWidget> = parseWidget(
constant("matrix"),
defaulted(constant("matrix"), () => "matrix"),
object({
prefix: string,
suffix: string,
prefix: optional(string),
suffix: optional(string),
answers: array(array(number)),
cursorPosition: array(number),
cursorPosition: optional(array(number)),
matrixBoardSize: array(number),
static: boolean,
static: optional(boolean),
}),
);
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import {
enumeration,
number,
object,
optional,
pipeParsers,
record,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseHint} from "./hint";
import {parsePerseusRenderer} from "./perseus-renderer";
Expand All @@ -18,14 +20,16 @@ import type {ParseContext, Parser, ParseResult} from "../parser-types";

export const parsePerseusItem: Parser<PerseusItem> = object({
question: parsePerseusRenderer,
hints: array(parseHint),
answerArea: pipeParsers(object({}))
hints: defaulted(array(parseHint), () => []),
answerArea: pipeParsers(defaulted(object({}), () => ({})))
.then(migrateAnswerArea)
.then(record(enumeration(...ItemExtras), boolean)).parser,
itemDataVersion: object({
major: number,
minor: number,
}),
itemDataVersion: optional(
object({
major: number,
minor: number,
}),
),
// Deprecated field
answer: any,
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,37 +1,32 @@
import {
array,
number,
object,
optional,
record,
string,
} from "../general-purpose-parsers";
import {array, object, optional, string} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseImages} from "./images-map";
import {parseWidgetsMap} from "./widgets-map";

import type {PerseusRenderer} from "../../../perseus-types";
import type {Parser} from "../parser-types";

export const parsePerseusRenderer: Parser<PerseusRenderer> = object({
content: string,
// This module has an import cycle with parseWidgetsMap, because the
// `group` widget can contain another renderer.
// The anonymous function below ensures that we don't try to access
// parseWidgetsMap before it's defined.
widgets: defaulted(
(rawVal, ctx) => parseWidgetsMap(rawVal, ctx),
() => ({}),
),
metadata: optional(array(string)),
images: defaulted(
record(
string,
object({
width: number,
height: number,
}),
export const parsePerseusRenderer: Parser<PerseusRenderer> = defaulted(
object({
// TODO(benchristel): content is also defaulted to empty string in
// renderer.tsx. See if we can remove one default or the other.
content: defaulted(string, () => ""),
// This module has an import cycle with parseWidgetsMap, because the
// `group` widget can contain another renderer.
// The anonymous function below ensures that we don't try to access
// parseWidgetsMap before it's defined.
widgets: defaulted(
(rawVal, ctx) => parseWidgetsMap(rawVal, ctx),
() => ({}),
),
() => ({}),
),
});
metadata: optional(array(string)),
images: parseImages,
}),
// Default value
() => ({
content: "",
widgets: {},
images: {},
}),
);
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
optional,
string,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";

import {parseWidget} from "./widget";
import {parseWidgetsMap} from "./widgets-map";
Expand All @@ -19,7 +20,7 @@ export const parseRadioWidget: Parser<RadioWidget> = parseWidget(
object({
choices: array(
object({
content: string,
content: defaulted(string, () => ""),
clue: optional(string),
correct: optional(boolean),
isNoneOfTheAbove: optional(boolean),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ export function parseWidget<Type extends string, Options>(
alignment: optional(string),
options: parseOptions,
key: optional(number),
version: object({
major: number,
minor: number,
}),
version: optional(
object({
major: number,
minor: number,
}),
),
});
}
Loading

0 comments on commit 3dbca96

Please sign in to comment.