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

Update comp-builder to run tests directly #168

Merged
merged 1 commit into from
Jun 7, 2019

Conversation

ruhatch
Copy link
Contributor

@ruhatch ruhatch commented Jun 7, 2019

This PR aims to get around several issues we've been experiencing running test suites with cabal. Instead of using $SETUP_HS to run the test executables in checkPhase, we run them directly.

To allow for test wrappers, such as wine, required on some systems, we add a testWrapper option to the package options. This will then be set in iohk-nix/mingw_w64.nix instead of setupTestFlags to get the same wine wrapper in place.

@ruhatch ruhatch requested review from angerman and rvl June 7, 2019 10:54
@ruhatch ruhatch force-pushed the ruhatch/test-exe branch from ac52eff to d320d44 Compare June 7, 2019 15:24
Copy link
Collaborator

@angerman angerman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm

@michaelpj
Copy link
Collaborator

This is what haskell/cabal#5995 is for, right? I guess we don't have a cabal release with that in yet. It would be good to have a comment that we should remove this one we can use that cabal option.

@ruhatch
Copy link
Contributor Author

ruhatch commented Jun 7, 2019

@michaelpj we were using that flag before, but cabal was being problematic (not consuming stdout fast enough, throwing weird file descriptor errors, etc), so we wanted to call the executables directly, while retaining a test wrapper option.

@ruhatch ruhatch merged commit 83e4c75 into input-output-hk:master Jun 7, 2019
@ruhatch ruhatch deleted the ruhatch/test-exe branch June 7, 2019 15:47
@angerman
Copy link
Collaborator

angerman commented Jun 8, 2019

@michaelpj, yes that‘s what I initially implemented to allow using cabal test. With component builds we know the text executable and can execute it directly instead of going through cabal and having to deal with the bugs that seem to be stdout/stderr redirection.

It would be great if we could fix this in cabal proper. I did look into this, hoping it could be fixed quickly, but didn’t get anywhere fast.

The approach taken here seems to actually even be superior to running it through cabal, as it removes one layer of indirection.

@rvl
Copy link
Contributor

rvl commented Jun 9, 2019

In Haskell.nix, I think it would be good if the derivation that executes a test suite were separated from the derivation that builds the test suite. (Sometimes we might want to build the tests but run them only nightly, or sometimes we might not want to bother with tests under wine, and instead just execute them on a real windows 10 VM).

And it would also be good to have a way of generically overriding all component derivations. For example, if hostPlatform = windows and componentType = test then override the checkPhase so that tests are run under wine.

@angerman
Copy link
Collaborator

angerman commented Jun 9, 2019

@rvl right that splitting would be a great addition. But seems orthogonal to this implementation.

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.

4 participants