Remove scoreInput
#1973
Merged
Remove scoreInput
#1973
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (4958a63) and published it to npm. You Example: yarn add @khanacademy/perseus@PR1973If you are working in Khan Academy's webapp, you can run: ./dev/tools/bump_perseus_version.sh -t PR1973 |
|
Size Change: -410 B (-0.03%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
handeyeco
commented
Dec 10, 2024
| @@ -118,31 +118,6 @@ describe("server item renderer", () => { | |||
| expect(screen.getByRole("textbox")).toBeVisible(); | |||
| }); | |||
|
|
|||
| it("should be invalid if no input provided", async () => { | |||
There was a problem hiding this comment.
I don't like deleting tests, but these seem to be specifically focused on testing SIR's ability to score.
| @@ -165,9 +165,6 @@ function scoreExpression( | |||
| // We matched a graded answer form, so we can now tell the user | |||
| // whether their input was correct or incorrect, and hand out | |||
| // points accordingly | |||
| // TODO(eater): Seems silly to translate result to this | |||
There was a problem hiding this comment.
idk, I think we're past the point of doing anything about this since this is from the Eater era.
jeremywiebe
approved these changes
Dec 11, 2024
## 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
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Remove
scoreInputfrom ServerItemRenderer. This was marked as deprecated and uses of it in Webapp have been removed in favor ofscorePerseusItem.Test plan:
This was an external-facing API and usage was already removed. At this point it's just dead code, so I'm not sure what would need to be tested.