Skip to content
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

Eliminate double eval from xtmtest and xtmtest-compile #386

Merged
merged 7 commits into from
May 18, 2020

Conversation

pdcawley
Copy link
Contributor

See #385 for the 'why' of this. This pull requests removes the double evaluation weirdness from xtmtest and xtmtest-compile and removes the 'just quote the form argument' workaround from the tests.

The docs will need updating to reflect that it's no longer necessary to quote the bind-func func when calling xtmtest.

Adds an xtmtest-with-fixture macro that sets up a fixture and then
runs its body in the fixture's environment.

There's a bunch of stuff introducing conveniently named macros within
the body, which got added as I was debugging and getting the basic macro
to work and it's a tad tricky to unpick this and present it as a series of
patches adding small pieces of behaviour every time. Sorry.
I thought, I'll fix the double eval in xtmtest and xtmtest-compile by
checking if the `form` argument is quoted, stripping the quote, and then
doing `(eval ',form (interaction-environment))` in the body of the test.

By my understanding of the way lisps work, this should not change behaviour in
any way.

Hah! Tests are now failing. What on _earth_ am I doing wrong?

squash! Fail a sanity check
We're still doing a double eval, but that's only until we fix the tests.
Since we've removed the double eval from xtmtest and xtmtest-compile,
we can now fix all the tests to stop quoting their 'form' arguments.

'tests/all-core.xtm' is still passing.
Since we've fixed the tests so they don't quote the 'form' argument,
we can remove the belt and braces code that unquoted such forms before
expanding the test macros as well.
@benswift benswift merged commit 43a18ee into digego:master May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants