Add event channel to certification function#672
Add event channel to certification function#672koslambrou merged 4 commits intoIntersectMBO:mainfrom
Conversation
shlevy
left a comment
shlevy
left a comment
There was a problem hiding this comment.
Mainly reviewed with respect to interface, let me know if you'd like me to review implementation as well.
| | CrashToleranceTask | ||
| | WhitelistTask | ||
| | DLTestsTask | ||
| deriving (Eq, Show) |
There was a problem hiding this comment.
Can we get Enum (reflecting the actual run order) and Bounded? That way if more stages are added I can still give an overall X/Y number until I update my source.
There was a problem hiding this comment.
Also can we get something like renderCertificationTask :: CertificationTask -> Text/String that's nicer for formatting to the user, again in the case where this repo updates before I've had a chance to update source on my end?
There was a problem hiding this comment.
If QC runs in more than one phase, can we get something like hasQCTests :: CertificationTask -> Bool?
There was a problem hiding this comment.
I don't think formatting for the end user is the responsibility of this code, and would be much better suited somewhere else.
| data CertificationOptions = CertificationOptions { certOptNumTests :: Int | ||
| , certOptOutput :: Bool } | ||
| data CertificationEvent = QuickCheckTestEvent (Maybe Bool) -- ^ Nothing if discarded, otherwise test result | ||
| | StartCertificationTask CertificationTask |
There was a problem hiding this comment.
Rather than start events, would it be difficult to switch to end events with a result? Then we can infer start from when the previous stage ends (or when I first call certify)
There was a problem hiding this comment.
What result are you looking for here? Would a boolean pass/fail be sufficient?
There was a problem hiding this comment.
Technically that information is already there in the QuickCheckTestEven constructor though?
There was a problem hiding this comment.
I suppose you don't get it for the unit tests though...
There was a problem hiding this comment.
I propose start and end events to make the consumer less dependent on the internals of this function. I've implemented this and I'll push it soon...
|
@koslambrou could we get this merged? |
4 participants
You can pass an optional
Control.Concurrent.Chanin the certification options to get events when tests pass or fail.Pre-submit checklist:
Tests are provided (if possible)