-
Notifications
You must be signed in to change notification settings - Fork 213
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
Code Coverage for integration tests scenarios #223
Conversation
So I suppose that the downside is that we no longer have a |
@piotr-iohk Indeed, but that's something we can re-enable fairly easily to make sure we still have some automated verification for this. |
Should I maybe do it in this PR directly? |
sure, why not :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm 👍 (as there are some discussions around cardano-wallet-launcher
, it can be done in another PR if needed)
3aecf52
to
fc9df49
Compare
@piotr-iohk Added a "Cardano.LauncherSpec" which just call the launcher using the command-line, and wait for a bit. If the launcher crashes or doesn't start, the test fails. |
cancel handle | ||
Right _ -> | ||
expectationFailure | ||
"cardano-wallet-launcher isn't supposed to terminates. \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, typo (terminates
)
Before, we ran this in a separate process which, as a major downside, has been escaping any form of coverage measurement. Having it ran in another _thread_ solves this problem
fc9df49
to
b053b11
Compare
Issue Number
#220
Overview
Comments
Note: I've also removed the 'Local' network target as it's not really a thing and isn't much use anyway.