search

exercism / go-test-runner

GNU Affero General Public License v3.0
15 stars 17 forks source link

Include errors from stderr for broken build output #41

Closed junedev closed 3 years ago

junedev commented 3 years ago

Fixes #39 Fixes #40

I manually checked all the cases mentioned in those tickets. All showed proper error messages now.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 1276004882

Totals Coverage Status
Change from base Build 1240411465: 0.0%
Covered Lines: 23
Relevant Lines: 29
💛 - Coveralls
junedev commented 3 years ago

@jmrunkle There is a second part to fix some other cases. I changed the PR description so the issue stays open. Now this can be merged as is, not sure who can do that though.

I will make another PR for the second part then.

jmrunkle commented 3 years ago

not sure who can do that though.

I think @ErikSchierboom or @iHiD can approve and merge it.

junedev commented 3 years ago

@jmrunkle Sorry, changed my mind and put this back to draft. What needs to be done for the other cases overlaps with this code.

junedev commented 3 years ago

Done. All cases from the issues should be covered now.

jmrunkle commented 3 years ago

Just verified that maintainers-admin is precisely @iHiD and @ErikSchierboom - is it correct that these should always require approval by those two?

iHiD commented 3 years ago

is it correct that these should always require approval by those two?

For test-runner stuff, yes, it needs signoff by one of us, as the consequences of it going wrong are very large for the site (e.g. a whole track stops working, or some malicious code gets merge that turns our servers into bitcoin farms, etc). Also, I like to be awake and online when tooling things are merged in case they have some negative effort during deploy (e.g. one the other day took down all our tooling servers because it caused them to run out of HDD space as there wasn't room on the machines for the extra images).

That said, for PRs written and reviewed by people we trust (e.g. everyone here) that don't change the Dockerfile, I normally just merge it without reading, which is what I'm about to do here :)