-
Notifications
You must be signed in to change notification settings - Fork 321
Add CI tests for bracket #467
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
Conversation
Revealed by #467. If the command "pipeline" in a Brench configuration actually has only a single command, then the current setup would crash because we would close the (sole) command's stdin before calling `communicate`. This little tweak fixes this with a special case for one-command pipelines.
|
Awesome. I was able to reproduce your instructions from the README and everything seems to work! One not-very-important aside: at first, I was getting a crash (with the message "I/O operation on closed file") when running Brench on your config. I'm not sure why this only happened for me, but I tracked it down to a bug in Brench. I've fixed that in #468. And a more-important suggestion: currently, your configuration file for Brench lives in a file called It is a little nonstandard to use Brench as a testing tool, exactly; it was originally meant for benchmarking only. But of course it has to do a little bit of differential testing just to check for optimizations that break correctness. From that perspective, this is a perfectly reasonable "abuse" of Brench and I don't object to it on principle. But anyway, with all that taken together, I think we should either:
I'm happy to go with either one! |
|
i see, i just updated |
It may look a little hacky, but the typical way to do differential testing with Turnt is just to set two different environments to share the same output suffix. So this collapses the `exprs_out` and `bril_out` outputs into a single `out` file per program, i.e., it enforces that both routes to execution produce the same output bytes.
|
Cool cool! I've tweaked this one step farther in the most recent commit, 38adb23. The idea is to avoid using two different output files ( Fortunately, everything still looks good: Anyway, let me know if that all looks good to you, and we can merge this! |
Yes, looks good! |
|
OK, cool! It is done! |
turnt.tomlused by brench to compare interpreter results (brili and exprs-lang's interpreter).