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

Start doing testing of compiler components #14

Closed
ollef opened this issue Oct 19, 2017 · 14 comments
Closed

Start doing testing of compiler components #14

ollef opened this issue Oct 19, 2017 · 14 comments

Comments

@ollef
Copy link
Owner

ollef commented Oct 19, 2017

Currently there are only end-to-end tests. We should have property and unit testing of smaller components in the compiler. An example is the simplifier which should ideally be idempotent (it's currently not, but testing that is the first step to solve that problem).

@typesanitizer
Copy link
Contributor

Is it this function that you're talking about?

Also, are you thinking about having an examples-based test or a property-based test for this? Any particular testing framework(s) to be used, such as tasty? Afaict, all the current tests are purely in Sixten and nothing in Haskell, so the testing script will also need to be modified/replaced.

@ollef
Copy link
Owner Author

ollef commented Nov 5, 2017

Yeah, that's the one!

I was thinking of a property based test, but thinking about it some more that would be fairly involved to get going, since we'll want to generate arbitrary well-typed terms. And an additional complication is that the function doesn't currently satisfy the property.

The right way to approach this is probably to get a trivial test, maybe for an Util function, running, and we can make a separate issue for testing simplification.

I like tasty since it allows both unit tests and property tests. If that's also your preference I say we should go for that. And you're right about the current testing situation.

@typesanitizer
Copy link
Contributor

By Util function, do you mean the ones in the Util module or in its submodules? I understood the simplifier part, but I'm not sure what property needs to be tested here ... could you give a concrete example?

I'm new to this but tasty's documentation seems nice, so I'm happy to use it. Would it be better to convert the existing tests first to run with tasty and then add new tests?

@typesanitizer
Copy link
Contributor

So I already started porting over the tests using Tasty.Golden :P. I've got the success tests working. It seems that about ~25 of those didn't have corresponding -expected files so tasty ended up automatically creating empty files. I will clean up the code and push soon-ish, so you can at least have a look and point out any glaring errors. Should I commit the newly generated empty files too?

After the first review's done, I'll write the rest of the code to incorporate the other existing tests; that part shouldn't take much effort.

@ollef
Copy link
Owner Author

ollef commented Nov 6, 2017

What I meant was that the important thing is to get some test up and running (and not try to do the more complicated simplification test at once) and was suggesting to look in the Util module for something trivial. Since you're porting the other tests there will already be some tests though, so it's not necessary. Sorry if I'm being unclear. :)

Running the existing tests with Tasty too sounds like a good idea. Probably easier than hacking shellscripts! Yeah, commit the empty files too.

Looking forward to seeing this!

@typesanitizer
Copy link
Contributor

Quick question: what is the "$@" supposed to do (ex. here)? I've seen it before in the context of makefiles, but I don't think it does anything in bash ...

@ollef
Copy link
Owner Author

ollef commented Nov 6, 2017

$@ is all the arguments given to test.sh, which means that it can be run e.g. as ./test.sh -v8 -O2 to set Sixten's verbosity level to 8 and optimisation to 2. It's occasionally useful so if it can be done with Tasty it would be nice, though it's not a must. :)

@typesanitizer
Copy link
Contributor

Okay, so here's the cleaned up version with the success tests working. typesanitizer@97aa8fa

I'll do the success multi tests next. For args, it seems like tasty always parses the command line args which is problematic...

@ollef
Copy link
Owner Author

ollef commented Nov 7, 2017

Looks great so far! Skip the arguments for now if it's problematic.

@typesanitizer
Copy link
Contributor

So I did figure out how to get the options working, the command to run is a bit verbose though:

stack test --ta='--sixten-args='-v8''

This is because Tasty tries to capture/use all arguments, so I have to create a dummy argument there, and then forward it to sixten.

Also, that makes the tests fail because of the verbosity 🙁. So I will replace the 'run' command with the 'test' command and just check the exit code instead of running golden tests (which compare stdin with the golden file, oops...).

Furthermore, there is something funny going atm where both inner and outer quotes have to be identical for the above command test to work, otherwise it spews a bunch of crap.

Lastly, I noticed something odd: supplying some flags to sixten run makes it crash.

$ sixten run -O2 tests/success/00-basics/Int.vix
sixten: opt: callProcess: runInteractiveProcess: exec: does not exist (No such file or directory)
 $ sixten run -o a.out tests/success/00-basics/Int.vix
sixten: a.out: callProcess: runInteractiveProcess: exec: does not exist (No such file or directory)

The compile command seems to be working fine though, so I don't get what's wrong ... you should be able to reproduce this on b4968e8.

@ollef
Copy link
Owner Author

ollef commented Nov 8, 2017

That way of passing flags seems alright to me (though the quirks seem a bit weird). Having it work with -v might not be strictly necessary, but sometimes testing at different -O levels can reveal bugs so that would be good to have.

The last thing is probably that the LLVM binary opt, used when you supply -O2 to sixten, has a different name on your machine like the other LLVM binaries.

@typesanitizer
Copy link
Contributor

Ooh, you're right! I didn't realize that opt was a binary. 😅 That said, don't you think the -o flag behaviour with run is incorrect? It does end up creating the correct binary, but emits an error saying it couldn't find the binary...

@ollef
Copy link
Owner Author

ollef commented Nov 9, 2017

Ah, good catch! Should be fixed by aa3b6b8. The problem was that it was using a relative path to the file, but I guess callProcess would then look for a system binary and not in the current working directory.

@ollef
Copy link
Owner Author

ollef commented Nov 15, 2017

We have the setup to do this now thanks to @theindigamer, so it's just a matter of starting to do it when it makes sense. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants