add fetch test from http #145
Conversation
| @@ -36,6 +38,10 @@ func TestCommand(testPath string, ctx TestCommandContext) error { | |||
| fmt.Println("Starting test against directory: " + testPath + "...") | |||
There was a problem hiding this comment.
What happens if ctx.Dir is true and the path is an URL?
There was a problem hiding this comment.
Hmmm, I believe the error will get caught in testDir, I'll check what the error message is. What error is it would be the same for file as well. I believe I already handled that. Worse case would you be fine with something like The input is not a directory?
There was a problem hiding this comment.
files, err := ioutil.ReadDir(directory) will respond with
commander (fetch-test-suites-over-http) $ ./commander test --dir https://raw.githubusercontent.com/commander-cli/commander/master/examples/commander.yaml
Starting test against directory: https://raw.githubusercontent.com/commander-cli/commander/master/examples/commander.yaml...
open https://raw.githubusercontent.com/commander-cli/commander/master/examples/commander.yaml: no such file or directory
and
./commander test --dir examples/commander.yaml Starting test against directory: examples/commander.yaml...
Hello
fdopendir: not a directory
I think a better error here would be
commander (fetch-test-suites-over-http) $ ./commander test --dir example/
Starting test against directory: example/...
Error: Input is not a directory
commander (fetch-test-suites-over-http) $ ./commander test --dir https://raw.githubusercontent.com/commander-cli/commander/master/examples/commander.yaml
Starting test against directory: https://raw.githubusercontent.com/commander-cli/commander/master/examples/commander.yaml...
Error: Input is not a directory
commander (fetch-test-suites-over-http) $ ./commander test --dir examples/commander.yaml Starting test against directory: examples/commander.yaml...
Error: Input is not a directory
commander (fetch-test-suites-over-http) $ ./commander test --dir -
Starting test against directory: -...
Error: Input is not a directory
There was a problem hiding this comment.
That looks good, thank you!
|
This looks good to me, reading from |
|
Your code looks good to me, looking forward for the tests 👍 |
dylanhitt
force-pushed
the
fetch-test-suites-over-http
branch
from
4bf9895 to
f53aa81
Compare
|
Hey @SimonBaeumer, I finished adding tests. I'm currently targeting a file that's already in master so I can demonstrate the integration test. I plan to switch over to One important note is that I did add a testing dependency - gock, in order to mock the http request. I'll add url support to the docs tomorrow. I would like to have this merged, and implement stdin in a separate PR, mainly just to switch the integration test file over to Best |
|
Your pull request looks great! Let me know when you are finished, would love to merge this! |
|
Done! |
|
Code Climate has analyzed commit f32aa9f and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 94.4% (50% is the threshold). This pull request will bring the total coverage in the repository to 92.5% (0.0% change). View more on Code Climate. |
|
merge :) |
Partially implements #109. @SimonBaeumer reading from stdin could come from another PR or I could bundle it in to this one. I think after stdin I may shoot over some prototypes for some possible refactors of
test_command.go. It feels a little strange passingtestPath,ctx.Filters, andfileNamebetween many different functions - this could also be addressed in #141 as well.I'll add some test by the end of the week.
Checklist
Linux,WindowsandmacOS?