-
Notifications
You must be signed in to change notification settings - Fork 283
(#1892) QA Runner improvements for use in CI #1910
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
34c0418
to
8d93a08
Compare
Add qa build script Update scripts
Exit bash script if failure
8d93a08
to
dfffbeb
Compare
ae77430
to
d45f9e1
Compare
Optimized so less volatile changes are done first (to take advantage of docker's image caching when possible) and changed the default command to use the Makefile targets for QA test suite.
d45f9e1
to
68d1e97
Compare
OPENBAZAARD_NAME ?= openbazaard-$(GIT_TAG)-$(GIT_SHA) | ||
BITCOIND_PATH ?= . | ||
|
||
.PHONY: openbazaard |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't a phony make target.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was concerned that the build output makes an openbazaard-<GITTAG>-<GITSHA>
file... not openbazaard
. Does that matter? (Not a makefile expert.)
Dockerfile.dev
Outdated
mv ./include/* /usr/local/include/ && \ | ||
mv ./bin/protoc /usr/local/bin/ && \ | ||
rm -rf ./include ./bin | ||
FROM docker.dev.ob1.io/openbazaar-qa:0.10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if anyone not authenticated against our registry will be able to pull this image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. @tyler-smith ?
220c6e4
to
68d1e97
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I refactored the Dockerfiles together and moved the qa_test.sh into the Makefile. The new Dockerfile defs seem to properly allow the container entrypoint to be executed as expected. I do see some cases where the make qa
task ends without running the tests as follows:
make: Nothing to be done for 'qa'.
Setting status of 4f0a6b6a32df7c4959a83f89626bc3f3303589e4 to SUCCESS with url https://jenkins/job/openbazaar-qa/98/ and message: 'Build finished. '
Finished: SUCCESS
It seems the .PHONY
call is not working in our jenkins environment? Might need to either enforce that make is built with support for .PHONY
or not use Makefile. Ideas on this or maybe a misdiagnosis? @szollo @tyler-smith
Discussion with @szollo suggests that we change the docker repo that holds openbazaar-qa and openbazaar-dev to be on the public Docker Hub repo instead of our internal repo. Will apply that change and get another review on this. |
112ef76
to
91d9ffc
Compare
Closing and opening to trigger jenkins build... |
Trigger another build... |
It always includes the most recent tag regardless if the SHA derives from that tag or not and is not representative of what the binary was built from.
Fixes #1892
Now pulls in code that is triggering the QA test suite and also now exits out of the sub-scripts if there is a unit test failure.