Orderer: Extract validation out of scoring #1869
Conversation
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (1e6f37f) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1869If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1869 |
|
Size Change: +105 B (+0.01%) Total Size: 1.29 MB
ℹ️ View Unchanged
|
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
| @@ -56,20 +56,4 @@ describe("ordererValiator", () => { | |||
| // assert | |||
| expect(result).toHaveBeenAnsweredIncorrectly(); | |||
| }); | |||
|
|
|||
| it("is invalid when the when the user has not started ordering the options and current is empty", () => { | |||
There was a problem hiding this comment.
I think it is worth retaining a unit test for the scorer that makes sure the score is "invalid" for this case. This ensures we're correctly calling and handling the result of the validator in the scorer.
There was a problem hiding this comment.
Added this back :) Do you think all scoring functions should retain a test confirming it can handle validation results? Also, I remember hearing about how it's ideal to make sure tests don't overlap so that multiple tests don't fail as a result of one piece of code failing (for example, validation). Is that why you mocked this out in your PRs?
There was a problem hiding this comment.
Updated the test to mock the validation and added a test confirming scoring handles passing validation with validation mocked.
Summary:
To complete server-side scoring, we are separating out validation logic from scoring logic. This separates that logic and updates associated tests.
Issue: LEMS-2603
Test plan: