Branch for working on rust support #12 #28
Conversation
| if (result.isPresent()) { | ||
| return parseResult(result.get(), dir); | ||
| } | ||
| return EMPTY_FAILURE; |
There was a problem hiding this comment.
Not sure how it is currently done elsewhere but returning empty value instead of error/exception omits the error (or is this just backup solution...)
There was a problem hiding this comment.
Not sure what you mean here.
There was a problem hiding this comment.
How it would look to the end user if the cargo test fails
There was a problem hiding this comment.
cargo test failing here would mean that there was an IOException while running the command. I am not sure what situation this would happen that wouldn't already happen while building tests. And compilation output when compilation failure happens is already preserved.
Dunno how to how to deal this.
|
Please add rust and other deps to travis. |
| return PARSING_FAILED; | ||
| } | ||
|
|
||
| private Optional<List<String>> parseResults(String[] lines) { |
There was a problem hiding this comment.
Why Optional if we never return Optional.absent(): can't we just return the empty list?
|
|
||
| #[test] | ||
| fn it_shall_work() { | ||
|
|
There was a problem hiding this comment.
Is this function block empty for purpose? I do not know Rust but it seems weird.
There was a problem hiding this comment.
Test fails when there is a panic (that wasn't expected). In this case I am testing that if student adds their own test that has same name as test for the exercise that it doesn't break.
There was a problem hiding this comment.
Let's comment it here (what is tested if it's not obvious)
|
|
||
| assertEquals(RunResult.Status.COMPILE_FAILED, result.status); | ||
| assertTrue(result.logs.containsKey(SpecialLogs.COMPILER_OUTPUT)); | ||
| assertTrue(new String(result.logs.get(SpecialLogs.COMPILER_OUTPUT)).contains("aborting due to previous error")); |
|
Added a few more nits, after those: LGTM. |
|
All seems to be fine, LGTM, Nice work! |
#18, #17 and #16