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

Oddness in the test suite #385

Open
pdcawley opened this issue May 16, 2020 · 5 comments
Open

Oddness in the test suite #385

pdcawley opened this issue May 16, 2020 · 5 comments

Comments

@pdcawley
Copy link
Contributor

So… I'm pretty sure there's a double evaluation bug in the testing library. It looks like the test suite is working around this bug because each call to xtmtest looks like:

(xtmtest '(setup-a-fixture-usually-by-binding-a-func)
  form-that-evaluates-to-a-number-or-other-self-eval-value #98 

Quoting the setup protects from the double eval, and the call part always evaluating to a simple constant protects that too. The test macros are also unhygenic and the test suite is written with that assumption. There's lots of code like:

(xtmtest `(bind-func test_something (lambda (x) ...))
  (test_something 1) #t)
(xtmtest-result (test_something 2) #372 

This means we have to be very careful about naming test functions and the like or test suites could interfere with each other. So, I propose the following (and there will be pull requests incoming)

  1. Writing a new xtmtest-with-fixture macro that would work something like:
(xtmtest-with-fixture 
  (bind-func test_list_membership
            (lambda (x)
              (let ((lst (list 1 2 3))
                    (result (member x lst)))
                (if (not (null? result))
                    (println "list contains" x)
                    (println "list does not contain" x))
                (not (null? result)))))
  
  (test_list_membership 2) 1)
  (test_list_membership 4) 0))
  1. Test this new macro
  2. Rewrite the test suite to use this new macro
  3. Eliminate xtmtest (and potentially rename xtmtest-with-fixture back to xtmtest
  4. Fix double evaluation in xtmtest-compile
@pdcawley
Copy link
Contributor Author

I'm raising this as an issue rather than just slinging pull requests because it's a bigger issue than i first thought, and it involves redesigning something, so it needs proper discussion, I think. I should have a first cut of xtmtest-with-fixture by close of play today.

@benswift
Copy link
Collaborator

Ha! "Oddness" is a nice euphemism---I don't doubt that there are some lurking bugs in there with all the evals and unhygenic macros and whatnot. I wrote all that testing code one night in a San Francisco hotel room after getting fed up with having to run the same ad-hoc test files over and over again.

If you want to re-design it in a more principled way then I'm happy to accept the PR. I think you have enough perspective on the various requirements of the combined Scheme/xtlang test infrastructure that I'd trust your choices on how best to re-design it. So go for it :)

@pdcawley
Copy link
Contributor Author

I’m less confident of my understanding of scheme/xtlang interactions I’m afraid. I only just discovered that bind-func appears to be a flat namespace, for instance

@pdcawley
Copy link
Contributor Author

pdcawley commented May 17, 2020 via email

@pdcawley
Copy link
Contributor Author

Ah... found out why it looked like I was losing my marbles – I hadn't noticed that xtmtest-compile was making an assumption about the shape of form that was no longer valid. Once that's fixed, tests/all-core.xtm passes again. However, I discovered that tests/core/builtins.xtm doesn't pass its tests (and didn't before my patch, so at least it's consistent). Is it known why?

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

No branches or pull requests

2 participants