-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fix test event listener for nested and unlabeled tests #2
Conversation
workspace/test.ml
Outdated
@@ -3,24 +3,51 @@ open OUnit | |||
let _esc_lf s = | |||
s |> Str.global_replace (Str.regexp_string "\n") "<:LF:>";; | |||
|
|||
(* TODO Fix missing `<COMPLETEDIN::>` *) | |||
let cw_print_success = function | |||
| _ -> print_endline ("\n<PASSED::>Test passed") |
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.
function
should only be used when there is a pattern-matching for the last argument. This function (and many other functions) should be written as
let cw_print_success () = print_endline "\n<PASSED::>Test passed"
Unnecessary parentheses should also be removed.
workspace/test.ml
Outdated
end | ||
| suite -> perform_test cw_print_test_event suite | ||
|
||
and dispatch_test_case = function |
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.
Don't create mutually recursive functions here (and
).
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.
They are mutually recursive tho, did not get a good idea yet how to decouple them.
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.
Then everything is fine here. I missed run_tests
in dispatch_test_group
.
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.
Btw this mutually recursive functions construct struck me as odd. I know some languages need it, but majority is fine with just some declaration and no special marker for recursive, or mutually recursive, functions.
Would you have any insight why ocaml, fsharp, maybe some other languages, need it? Is it just syntax, or is it relevant from compilers point of view, or anything else?
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.
It is just syntax. The only real difference is that functions without rec
(or and
) can shadow functions (or constants) with the same name defined earlier and call old functions in their body. Compiling non-mutual recursive functions may be a little faster also.
workspace/test.ml
Outdated
| suite -> perform_test cw_print_test_event suite | ||
|
||
and dispatch_test_case = function | ||
| label, test_case -> begin |
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.
Instead of pairs (label, test_case
) it is better to use separate arguments:
let dispatch_test_case label test_case =
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.
Yeah, randomly spraying parentheses and semicolons is my way of dealing with syntax errors and type mismatches.
I am noob.
Can I edit this PR somehow? Everything works fine but some small changes are required to make the code cleaner. |
You can even steal the code to your repo. |
I made you a collaborator in my repo, but I dont know if it helps anything :) |
I will push my changes to your repository. Then you will be able to update this PR. |
@monadius I gave you |
I successfully pushed my changes to this PR. |
Fixes #1 .
Supports:
Example kumite: https://www.codewars.com/kumite/620a7361d746e5000f36a021/
Test output:
Current output panel:
Output panel with updated listener: