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

Tests are horribly out-of-date #103

Closed
goldfirere opened this issue Jan 6, 2015 · 10 comments
Closed

Tests are horribly out-of-date #103

goldfirere opened this issue Jan 6, 2015 · 10 comments
Labels

Comments

@goldfirere
Copy link
Owner

The testsuite just failed 36 out of 40 tests. I don't think it's been updated since allowing singletonization of overlapping patterns.

@goldfirere goldfirere added the bug label Jan 6, 2015
@jstolarek
Copy link
Collaborator

I remember trying to fix broken singletonization of Nat (see #97). I think that for some reason I found this non-trivial but can't remember why exactly :-/ Anyway, I'm totally busy with teaching until the end of month. After that I can try and help with this.

Aside: I found updating testsuite results a pain. Mostly because copying the test results to be new golden files was tedious. That's why I was thinking about creating a script that would take all the results from the testsuite run and use them to create new golden files. Never got to actually writing it though. Perhaps now is a good moment for this.

@goldfirere
Copy link
Owner Author

Not a terrible idea, but it would require care to use. As annoying as updating the golden files is, I've found real bugs by reading diffs, even when I thought about just copying over. Doing it by hand forces me to think about each file, and that may be a good thing.

@jstolarek
Copy link
Collaborator

Obviously yes, you have to examine the diffs carefully. The script would just simplify updating the results.

@jstolarek
Copy link
Collaborator

Richard, I'm afraid this one is mostly on you as after th-desugar changes I understand little of generated code - see my comment in #97.

Now that we support two versions of GHC - 7.8 and 7.10 - we can have different code generated by each of these versions. Luckily the testsuite supports that. My proposal is this: if you fix the tests on GHC 7.8 I'll do all the boring work of adding support for GHC 7.10. Does that sound good?

@goldfirere
Copy link
Owner Author

This is effectively blocked by #113. There's not much point in fixing tests until we decide if match flattening is effective.

@jstolarek
Copy link
Collaborator

I strongly disagree. This "temporary" test breakage lasts for months now. Tests are meant to ensure that changes that we make to the library don't introduce any unexpected behaviour. If I make changes and they break something I won't even realize. Sure that the current implementation of flattening might not be our final word but I don't think this is the reason to have broken tests. If we change flattening in the future we'll update the tests again.

@goldfirere
Copy link
Owner Author

I agree with your sentiment but not your conclusion: I think the current state of affairs calls for rolling back match flattening. Perhaps we should revert fabe02a and 4610fc5, and then proceed. I can then reapply these commits on a branch, sort out the problems, fix the tests, and merge. In the meantime, with match flattening reverted, we can make forward progress.

Thoughts?

@jstolarek
Copy link
Collaborator

I think the current state of affairs calls for rolling back match flattening.

Perhaps. I don't have a strong opinion here. If it is meant to be an experimental feature and the price for having it in the master branch is a broken testsuite then it should most likely go into its own separate branch.

@goldfirere
Copy link
Owner Author

I have removed match flattening. The tests work with 7.8. Can you update for 7.10, which I recall was your offer? Thanks!

@jstolarek
Copy link
Collaborator

I updated the tests. Turns out that some things don't work under GHC 7.10. See #116, #117 and #118.

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

No branches or pull requests

2 participants