Add blackbox tests for serve command#608
Conversation
aelsabbahy
left a comment
aelsabbahy
left a comment
There was a problem hiding this comment.
Looks good, just needs to be hooked into CI
| $(info INFO: Starting build $@) | ||
| go test -bench=. | ||
|
|
||
| alpha-test-%: release/goss-% |
There was a problem hiding this comment.
via ci/build.sh in the if darwin or windows bit.
(Honestly, Makefiles are not a strong suit for me, so I haven't wired there)
| $(info INFO: Starting build $@) | ||
| ./integration-tests/run-tests-alpha.sh $* | ||
|
|
||
| test-serve-%: release/goss-% |
There was a problem hiding this comment.
make test should execute this as part of it's testing, so that it's part of CI.
I was able to test it locally with make test-serve-linux-amd64 and it looks great.
There was a problem hiding this comment.
It's wired via ci/build.sh - or have I made a mistake?
petemounce
force-pushed
the
serve-integration-test-coverage
branch
from
9983e32 to
fadc66a
Compare
@aelsabbahy I think that it already is via |
|
I'm fine with build.sh calling one make task, but I don't want build.sh to turn into an orchestration layer of multiple tasks or make calls. Basically, for local development a user should be able to run |
|
I've had a go, please take a look? It's complicated by needing to filter to the current OS that is being executed on, so it sounds like |
|
I'll take a look over the weekend. Thanks for making the changes. If you don't mind can you send me an order list of all the PRs that are ready to go and which order you prefer them merged in (if any). I haven't used bazel (outside or demoing it), but I love the concepts behind it and nixos for hermetic builds. |
| if curl --silent "http://127.0.0.1:${open_port}/healthz" | grep 'Count: 2, Failed: 0, Skipped: 0' ; then | ||
| echo "passed" | ||
| else | ||
| echo "failed, exit code $?" |
There was a problem hiding this comment.
Is this supposed to fail? It just echo's out a message and succeeds.
I added a:
ret=$?
exit $ret
but that didn't work because of the trap
maybe:
ret=$?
and a exit "$ret" at the bottom of cleanup?
There was a problem hiding this comment.
Yeah, that was supposed to fail. Fixed inside 23ce6f1. Good catch - sorry!
petemounce
force-pushed
the
serve-integration-test-coverage
branch
from
27f7049 to
23ce6f1
Compare
|
@aelsabbahy I've rebased and also fixed the lack of failure-on-tests-failing. |
|
Ci test failure appears related to this change. I think this PR is really close.. but maybe some intermittent issues? |
petemounce
force-pushed
the
serve-integration-test-coverage
branch
from
90770b5 to
dffb864
Compare
|
I've rebased and made no changes that I think could have fixed some problem, but it is green now. |
|
Restarted the build 3 times and it was green all three times, so it might be intermittent. |
2 participants
Checklist
make test-all(UNIX) passes. CI will also test thisDescription of change
Added a cross-platform blackbox test for
goss serveto give some outside in safety to #606 and incoming output-format-via-http-content-negotiation change.Also integrated the alpha testing into the
Makefilefor coherence with the others. I'm not a comfortable make-r, so please critique brutally if I've done things poorly?