Sorter: Extract validation out of scoring #1876
Conversation
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (61de24e) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1876If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1876 |
|
Size Change: +155 B (+0.01%) Total Size: 1.29 MB
ℹ️ View Unchanged
|
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
|
|
||
| it("is invalid when the user has not made any changes", () => { | ||
| const userInput: PerseusSorterUserInput = { | ||
| options: ["$15$ grams", "$55$ grams", "$0.005$ kilograms"], | ||
| changed: false, | ||
| }; | ||
| const rubric: PerseusSorterRubric = { | ||
| correct: ["$0.005$ kilograms", "$15$ grams", "$55$ grams"], | ||
| }; | ||
| const result = scoreSorter(userInput, rubric); | ||
| expect(result).toHaveInvalidInput(); | ||
| }); |
There was a problem hiding this comment.
I think we could keep a variant of this test in each scorer's test suite. But, we can use mocking to ensure that validate is called first... something like:
I think we could keep a variant of this test in each scorer's test suite. But, we can use mocking to ensure that validate is called first... something like:
// Top of file
import * as SorterValidator from './validate-sorter'
// Inside describe() block...
it("should score if validator passes", () => {
// Mock validator saying "all good" 👍
const mockValidate = jest
.spyOn(SorterValidator, "default")
.mockReturnValue(null);
const score = scoreSorter(userInput, rubric);
// Assert
expect(mockValidate).toHaveBeenCalled();
expect(score).toHaveBeenAnsweredCorrectly();
});
it("should abort if validator returns invalid", () => {
// Mock validator saying "all good" 👍
const mockValidate = jest
.spyOn(SorterValidator, "default")
.mockReturnValue({type: "invalid", message: null});
const score = scoreSorter(userInput, rubric);
// Assert
expect(mockValidate).toHaveBeenCalled();
expect(score).toHaveInvalidInput()
});
Unnecessary value since the validator is being mocked, but want to keep it consistent
| expect(result).toHaveBeenAnsweredIncorrectly(); | ||
| }); | ||
|
|
||
| it("is invalid when the user has not made any changes", () => { | ||
| it("should abort if validator returns invalid", () => { |
There was a problem hiding this comment.
@jeremywiebe Do the below added tests look as expected? Thanks!
There was a problem hiding this comment.
Yes, this looks correct! Thanks for adding these!
Matthew has an idea to refactor the scoring so that we don't have to guarantee the scorer calls validation first: https://khanacademy.atlassian.net/browse/LEMS-2658
But for now, let's leave these tests and logic in place.
Summary:
To complete server-side scoring, we are separating out validation logic from scoring logic. This separates that logic for the sorter widget and updates associated tests.
Issue: LEMS-2605
Test plan: