Remove `guessAndScore` #1974

handeyeco · 2024-12-10T16:50:53Z

Summary:

Remove guessAndScore from Renderer. This hasn't been used in learner-facing logic for a little bit now. The bulk of this PR is updating internal usage - tools for testing exercise correctness were using this (specifically in Cypress and our custom matchers for Jest); so most of the work was porting tests to use scorePerseusItem instead.

Test plan:

From a learner's perspective, this is dead code. So while this is a major API change, it's mostly removing dead code and updating tests. Tests should continue to run successfully.

github-actions · 2024-12-10T16:55:05Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (2fbfbf4) and published it to npm. You
can install it using the tag PR1974.

Example:

yarn add @khanacademy/perseus@PR1974

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR1974

github-actions · 2024-12-10T16:55:13Z

Size Change: -51 B (0%)

Total Size: 1.27 MB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	414 kB	-51 B (-0.01%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	39 kB
`packages/keypad-context/dist/es/index.js`	760 B
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	77.9 kB
`packages/math-input/dist/es/strings.js`	1.79 kB
`packages/perseus-core/dist/es/index.js`	1.48 kB
`packages/perseus-editor/dist/es/index.js`	688 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus/dist/es/strings.js`	3.7 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.5 kB

_{compressed-size-action}

handeyeco · 2024-12-10T21:43:00Z

config/test/custom-matchers.ts

-    return {widgetState, score};
-}
-
-function maybeAddState(message: string, widgetState: string): string {


I removed widgetState from the tests and they didn't seem to care. So I'm thinking this is a feature we didn't think was particularly useful. Also since the Renderer isn't scoring now, the correct/incorrect helper isn't the place to get that.

handeyeco · 2024-12-10T21:45:11Z

packages/perseus/src/renderer.tsx

-    /**
-     * @deprecated use scorePerseusItem
-     */
-    guessAndScore: () => [UserInputArray, PerseusScore] = () => {


Happy about this, but I wanted to go further and delete score. Unfortunately I'm not sure we're ready for that until we have a path for client-side validation.

handeyeco · 2024-12-10T21:45:39Z

packages/perseus/src/util/test-utils.ts

+    perseusRenderData: PerseusRenderer,
+    userInputMap: UserInputMap,
+): PerseusScore {
+    return scorePerseusItem(perseusRenderData, userInputMap, mockStrings, "en");


I just didn't want to bother with importing mockStrings everywhere.

Praise: I love this technique! Trying to get it into my memory banks so I can use it sometime too. Very cool.

handeyeco · 2024-12-10T21:46:35Z

packages/perseus/src/widgets/grapher/grapher.cypress.ts

+                    absoluteValueQuestion,
+                    userInput,
+                );
+                expect(score).toStrictEqual({


Cypress doesn't have access to toHaveBeenAnsweredCorrectly AFAIK

Comment: Good to know! Haven't written Cypress tests in a while.

That's correct, because toHaveBeenAnsweredCorrectly is a Jest custom matcher. Although our Cypress tests look kinda like Jest tests, they aren't, and they just happen to have similar assertion APIs. We didn't replicate the custom matchers for the Renderer in Cypress.

handeyeco · 2024-12-10T21:47:50Z

packages/perseus/src/__tests__/renderer-api.test.tsx

            // Assert
-            expect(renderer).toHaveBeenAnsweredCorrectly();
+            expect(score).toHaveBeenAnsweredCorrectly();


90% of the changes are just updating tests to score outside of Renderer and then using that score for the assertion.

Myranae

Thanks! Looks great! Went through and tested most of the widgets in SB and they all seem good too.

Myranae · 2024-12-11T14:36:39Z

packages/perseus/src/util/test-utils.ts

+    perseusRenderData: PerseusRenderer,
+    userInputMap: UserInputMap,
+): PerseusScore {
+    return scorePerseusItem(perseusRenderData, userInputMap, mockStrings, "en");


Praise: I love this technique! Trying to get it into my memory banks so I can use it sometime too. Very cool.

Myranae · 2024-12-11T14:46:46Z

packages/perseus/src/widgets/graded-group/graded-group.test.ts

-    // NOTE(jeremy): The graded-group widget does not participate in
-    // Renderer grading. So we can't call `renderer.score()` and see that
-    // the widget is answered correctly. The only route to check the answer


Question: Did you change this because no widgets will participate in renderer grading after this?

Right, since renderer.score is going away soon I didn't see the need to make that distinction here.

I filed a ticket to look into this more: https://khanacademy.atlassian.net/browse/LEMS-2719

Myranae · 2024-12-11T14:48:31Z

packages/perseus/src/widgets/grapher/grapher.cypress.ts

+                    absoluteValueQuestion,
+                    userInput,
+                );
+                expect(score).toStrictEqual({


Comment: Good to know! Haven't written Cypress tests in a while.

* bulk of it removed, tests passing * odds and ends * changeset * remove i18n * respond to Jeremys feedback * remove scoring things again * changeset * Remove `guessAndScore` (#1974) ## Summary: Remove `guessAndScore` from Renderer. This hasn't been used in learner-facing logic for a little bit now. The bulk of this PR is updating internal usage - tools for testing exercise correctness were using this (specifically in Cypress and our custom matchers for Jest); so most of the work was porting tests to use `scorePerseusItem` instead. ## Test plan: From a learner's perspective, this is dead code. So while this is a major API change, it's mostly removing dead code and updating tests. Tests should continue to run successfully. Author: handeyeco Reviewers: handeyeco, Myranae, jeremywiebe Required Reviewers: Approved By: Myranae, jeremywiebe Checks: ✅ Publish npm snapshot (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), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ⏹️ [cancelled] Publish npm snapshot (ubuntu-latest, 20.x), ⏹️ [cancelled] Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ⏹️ [cancelled] Check builds for changes in size (ubuntu-latest, 20.x), ⏹️ [cancelled] Cypress (ubuntu-latest, 20.x), ⏹️ [cancelled] Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #1974

handeyeco added 3 commits December 10, 2024 10:39

don't use renderer scoring for tests

38c58b7

update cypress tests

d0a2153

remove guessAndScore

e96c708

handeyeco self-assigned this Dec 10, 2024

handeyeco changed the title ~~don't use renderer scoring for tests~~ WIP: remove guessAndScore Dec 10, 2024

changeset

2fbfbf4

handeyeco changed the title ~~WIP: remove guessAndScore~~ Remove guessAndScore Dec 10, 2024

handeyeco requested review from a team December 10, 2024 21:40

handeyeco commented Dec 10, 2024

View reviewed changes

Myranae approved these changes Dec 11, 2024

View reviewed changes

jeremywiebe approved these changes Dec 11, 2024

View reviewed changes

handeyeco merged commit fcbde58 into remove-scoreinput2 Dec 11, 2024
13 of 18 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove `guessAndScore` #1974

Remove `guessAndScore` #1974

handeyeco commented Dec 10, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

handeyeco Dec 10, 2024

handeyeco Dec 10, 2024

handeyeco Dec 10, 2024

Myranae Dec 11, 2024

handeyeco Dec 10, 2024

Myranae Dec 11, 2024

jeremywiebe Dec 11, 2024

handeyeco Dec 10, 2024

Myranae left a comment

Myranae Dec 11, 2024

Myranae Dec 11, 2024

handeyeco Dec 11, 2024

handeyeco Dec 11, 2024

Myranae Dec 11, 2024

Remove guessAndScore #1974

Remove guessAndScore #1974

Conversation

handeyeco commented Dec 10, 2024 • edited Loading

Summary:

Test plan:

github-actions bot commented Dec 10, 2024 • edited Loading

npm Snapshot: Published

github-actions bot commented Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Myranae left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Remove `guessAndScore` #1974

Remove `guessAndScore` #1974

handeyeco commented Dec 10, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading