Testing Refactor #1707
Draft
Testing Refactor #1707
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
Replaces the depended functions with their equivalents.
* Remove interpreter (mostly) * Redirect testing to use ec-evaluator * Update snapshots * Update dependents of interpreter
…nto remove-interpreter
…-refactor # Conflicts: # src/__tests__/__snapshots__/scmlib.ts.snap # src/interpreter/closure.ts # src/interpreter/interpreter.ts # src/mocks/context.ts # src/repl/repl.ts # src/runner/sourceRunner.ts
This was
linked to
issues
May 22, 2024
RichDom2185
reviewed
May 22, 2024
There was a problem hiding this comment.
Wow, thanks a lot for this! I agree with the questions that are to be answered, let's wait for Prof to come back from his break.
I did a really really quick run through and give my in-principle approval – let's merge this PR if possible, I don't see why the SICP issue needs to be resolved first, since they can always retain an older version of js-slang in their package.json.
|
One more thing: I have no idea what Ideally I'd like to remove things like |
# Conflicts: # src/stdlib/__tests__/__snapshots__/pylib.ts.snap # src/stdlib/__tests__/pylib.ts
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.
TL; DR
svml-machine.tsjs-slangtests more robust and simplerWhat's going on:
Because our testing utilities relied heavily on the interpreter, removing it means touching almost every single part of
js-slang, hence the ridiculous number of lines changed. I've tried to simplify the testing utilities and refactor repetitive code where necessary without breaking anything I hope. For example, I noticed that the stepper's tests were being run as async tests whengetEvaluationStepsis no longer asynchronous. Hopefully with these changes it will make such things easier to catch.I've also configured
tsconfig.prod.jsonto excludesrc/utils/testing. Previously this code was being shipped with the rest ofjs-slang, but given that we only use it for unit testing I don't think it should be included whenjs-slangis being built.Given that @martin-henz has indicated that other parts of
js-slangare marked for removal, I decided to remove them here to avoid having to refactor the tests for those parts ofjs-slang.Please refer to the TODO comments to see what issues need to be addressed before this PR can be merged.
Will resolve these issues:
src/interpreter/interpreter-non-det.ts(currently not working anyway) #1695src/lazy#1696src/vm/svml-machine.ts#1697Some issues that need to be revisited: