Fixes for release_pass test #6572
Conversation
glevand
force-pushed
the
for-merge-release_pass
branch
2 times, most recently
from
fe01dbb to
08ba05f
Compare
| @@ -33,6 +33,20 @@ TEST_PKGS=`find . -name \*_test.go | while read a; do dirname $a; done | sort | | |||
| FORMATTABLE=`find . -name \*.go | while read a; do echo $(dirname $a)/"*.go"; done | sort | uniq | egrep -v "$IGNORE_PKGS" | sed "s|\./||g"` | |||
| TESTABLE_AND_FORMATTABLE=`echo "$TEST_PKGS" | egrep -v "$INTEGRATION_PKGS"` | |||
|
|
|||
| case "$(uname --machine)" in | |||
There was a problem hiding this comment.
this breaks on osx; stick with -m
| esac | ||
|
|
||
| tar xzvf /tmp/$file -C /tmp/ --strip-components=1 | ||
| mkdir -p ./bin |
There was a problem hiding this comment.
why do this? test should always run from the etcd repo root...
There was a problem hiding this comment.
As I mentioned in the commit message, if you run PASSES="release" ./test, the test will fail. This is because there is no bin directory in the etcd repo.
| local file="etcd-$UPGRADE_VER-linux-$MACHINE_ARCH.tar.gz" | ||
| echo "Downloading $file" | ||
|
|
||
| set +e |
There was a problem hiding this comment.
why bother with this and the extra error checking? it's simpler to let the non-zero exit take down the process; the output from the curl command should be enough error output
There was a problem hiding this comment.
A curl return of 22 (file not found) is success, so we need to check for that, and then in that case just return and not run tar.
If we keep ``set e` on, then curl's return of 22 will result in test failure.
| echo "Downloading $file" | ||
|
|
||
| set +e | ||
| curl --fail --progress-bar -L https://github.com/coreos/etcd/releases/download/$UPGRADE_VER/$file -o /tmp/$file 2>&1 |
There was a problem hiding this comment.
no progress bar? it'll mess up the output when piped to a file
There was a problem hiding this comment.
OK, I can remove it. I added that to cleanup the screen output when curl returns file not found.
| echo "Unknown machine" | ||
| exit 255 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
Is this code block necessary? Listing stuff out is tedious. How about if [ -z "$GOARCH" ]; then GOARCH=$(go env GOARCH); fi ? I think treating the GOARCH as the machine arch is fine for the test script.
| RACE="--race" | ||
| fi | ||
| elif [ "$GOARCH" == "amd64" ]; then | ||
| if [ "$GOARCH" == "amd64" ] || [[ -z "$GOARCH" && "$MACHINE_ARCH" == "amd64" ]]; then |
There was a problem hiding this comment.
this can just be if [ "$GOARCH" == "amd64" ]; then if you use the GOARCH suggestion
| curl -L https://github.com/coreos/etcd/releases/download/$UPGRADE_VER/etcd-$UPGRADE_VER-linux-amd64.tar.gz -o /tmp/etcd-$UPGRADE_VER-linux-amd64.tar.gz | ||
| tar xzvf /tmp/etcd-$UPGRADE_VER-linux-amd64.tar.gz -C /tmp/ --strip-components=1 | ||
|
|
||
| local file="etcd-$UPGRADE_VER-linux-$MACHINE_ARCH.tar.gz" |
There was a problem hiding this comment.
s/MACHINE_ARCH/GOARCH/ ?
glevand
force-pushed
the
for-merge-release_pass
branch
from
08ba05f to
8f2c943
Compare
|
Rebased to latest, addressed comments so far. |
|
lgtm after squashing. Thanks! |
Some fixes related to release_pass: o Create the output directory ./bin if it does not exist. o Define the GOARCH variable if it is not defined. o Simplify the race detection test. o Download the relese archive based on GOARCH. o If the release file is not found, return success. This will allow the tests to continue. Signed-off-by: Geoff Levand <[email protected]>
glevand
force-pushed
the
for-merge-release_pass
branch
from
8f2c943 to
25f1088
Compare
|
Rebased to latest, squashed to a single patch. |
No description provided.