-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Testing Refactor #1707
base: master
Are you sure you want to change the base?
Testing Refactor #1707
Conversation
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
TL; DR
svml-machine.ts
js-slang
tests 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 whengetEvaluationSteps
is no longer asynchronous. Hopefully with these changes it will make such things easier to catch.I've also configured
tsconfig.prod.json
to 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-slang
is being built.Given that @martin-henz has indicated that other parts of
js-slang
are 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: