exercism / elm-test-runner

GNU Affero General Public License v3.0
3 stars 5 forks source link

Fix crashes due to usage on `Debug.log` #6

Closed mpizenberg closed 4 years ago

mpizenberg commented 4 years ago

This fixes crashes happening when using Debug.log in the exercise code (fix #5). All logging is discarded. A better solution enabling students to view logged values would require more control over the testing framework elm-test.

mpizenberg commented 4 years ago

Yes, from my experience the junit xml report is always at the end of stdout stream. It is not the case if using --report json though but we don't really care. In that case debug lines and report lines are interleaved in an undetermined way.

mpizenberg commented 4 years ago

A question about procedures, is @ErikSchierboom approval ok for us to merge (here and in general for subsequent test-runner PRs), or should we wait for @ccare to merge? I remember Jeremy telling that Charles is in charge of test runners.

ErikSchierboom commented 4 years ago

@mpizenberg Good point! I'm actually not sure. @ccare @iHiD ?

iHiD commented 4 years ago

Whoever merges should deploy, basically. So that head is always deployed and we don't get unexpected failures later.

Both Erik and Charles have the bits for that. So as long as @ErikSchierboom is happy to merge, test and deploy it, then he can merge it.

If it's a change to the actual Dockerfile or similar, then it makes sense for @ccare to do it as he can see "deeper" logs than Erik or I currently have visibility on.

ErikSchierboom commented 4 years ago

Merged and deployed! Using Debug.log indeed doesn't crash the test runner.