search

exercism / go-test-runner

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

Refactored to run "go test" from within the test runner #17

Closed ekingery closed 3 years ago

ekingery commented 3 years ago

Also simplified source code structure, and removed the unused tests.

Of note - due to moving main.go, git makes it appear that I authored / changed the entire file. This is not the case, which is shown by 2535b838ca278e02c0f9867521a3a0d9356d9b12. I am open to leaving main.go in cmd/testout/ if we want to keep the history in place. I changed it because testout didn't seem apt, and in general I appreciate the simplicity of having a flat repo / code structure (while the codebase remains tiny). I embedded the Report and Test structs and removed the exreport package for the same reason.

iHiD commented 3 years ago

(@tehsphinx Not sure if you've met @ekingery, but he's been very active over the last couple of months across Exercism and has started working on the Go side of things for v3 - just to give some context for you!) 🙁

ekingery commented 3 years ago

Of note - running go test from within the test runner is in preparation for the upcoming changes to parse out the test_code field for V3. Discussion here: https://exercism-team.slack.com/archives/CC4UPN5KP/p1607203773004100?thread_ts=1607203773.004100&cid=CC4UPN5KP

ekingery commented 3 years ago

I did notice that no tests are being run in CI. I'd strongly suggest having a test suite and running it as part of the CI...

@ErikSchierboom I couldn't agree more. I am adding new tests for the new functionality in another branch (see 761772a1567eaeb0bae2d6dc2ee9e0c4d2c174cd). I decided to not add tests in this PR for two reasons. 1) the code is not currently test-friendly and 2) I wanted this PR to be "minimally invasive" and not touch existing functionality. The goal of this PR is simply switching from accepting stdin from the shell script to running go test internally. As mentioned above, unfortunately git makes it look like I rewrote the whole main.go file because I moved it and embedded the exreport structs, but in actuality the only changes I made to the existing code was to switch from a bytes array to a bytes buffer in 2535b838ca278e02c0f9867521a3a0d9356d9b12.

ErikSchierboom commented 3 years ago

@ekingery That totally makes sense! Just wanted to mention it :)