Skip to content

Comments

Various improvements to the testing shell scripts#1368

Open
wilzbach wants to merge 4 commits intodlang:masterfrom
wilzbach:dub-binary
Open

Various improvements to the testing shell scripts#1368
wilzbach wants to merge 4 commits intodlang:masterfrom
wilzbach:dub-binary

Conversation

@wilzbach
Copy link
Contributor

@wilzbach wilzbach commented Feb 10, 2018

See also: #1339

  • [CI]: Update setup-dlang GitHub org name
  • [test]: Make scripts more portable by not assuming bash location
  • [scripts/ci/travis.sh]: Error on unset bash variables
  • [scripts/ci/travis.sh]: Make host dub unavailable implicitly

@dlang-bot dlang-bot added the WIP label Feb 10, 2018
@dlang-bot
Copy link
Collaborator

Thanks for your pull request, @wilzbach!

@PetarKirov
Copy link
Member

Your first commit seems to have some merge problems (note the 2 re-arranged if-then-s):

diff --git test/fetchzip.sh test/fetchzip.sh
index 723e698..4bbac58 100755
--- test/fetchzip.sh
+++ test/fetchzip.sh
@@ -5,7 +5,7 @@ DIR=$(dirname "${BASH_SOURCE[0]}")
 
 PORT=$(($$ + 1024)) # PID + 1024
 
-dub remove gitcompatibledubpackage --non-interactive --version=* 2>/dev/null || true
+"$DUB" remove gitcompatibledubpackage --non-interactive --version=* 2>/dev/null || true
 
 "$DUB" build --single "$DIR"/test_registry.d
 "$DIR"/test_registry --folder="$DIR/issue1336-registry" --port=$PORT &
@@ -18,7 +18,7 @@ timeout 1s "$DUB" fetch gitcompatibledubpackage --version=1.0.4 --skip-registry=
 if [ $? -eq 124 ]; then
     die 'Fetching from responsive registry should not time-out.'
 fi
-dub remove gitcompatibledubpackage --non-interactive --version=1.0.4
+"$DUB" remove gitcompatibledubpackage --non-interactive --version=1.0.4
 
 echo "Downloads should be retried when the zip is corrupted - gitcompatibledubpackage (1.0.3)"
 zipOut=$(! timeout 1s "$DUB" fetch gitcompatibledubpackage --version=1.0.3 --skip-registry=all --registry=http://localhost:$PORT 2>&1)
@@ -32,8 +32,8 @@ if ! zipCount=$(grep -Fc 'Failed to extract zip archive' <<<"$zipOut") || [ "$zi
 elif [ $rc -eq 124 ]; then
     die 'DUB timed out unexpectedly.'
 fi
-if dub remove gitcompatibledubpackage --non-interactive --version=* 2>/dev/null; then
     die 'DUB should not have installed a broken package.'
+if "$DUB" remove gitcompatibledubpackage --non-interactive --version=* 2>/dev/null; then
 fi
 
 echo "HTTP status errors on downloads should be retried - gitcompatibledubpackage (1.0.2)"
@@ -47,8 +47,8 @@ if ! retryCount=$(echo "$retryOut" | grep -Fc 'Bad Gateway') || [ "$retryCount"
 elif [ $rc -eq 124 ]; then
     die 'DUB timed out unexpectedly.'
 fi
-if dub remove gitcompatibledubpackage --non-interactive --version=* 2>/dev/null; then
     die 'DUB should not have installed a package.'
+if "$DUB" remove gitcompatibledubpackage --non-interactive --version=* 2>/dev/null; then
 fi
 
 echo "HTTP status errors on downloads should retry with fallback mirror - gitcompatibledubpackage (1.0.2)"
@@ -56,4 +56,4 @@ timeout 1s "$DUB" fetch gitcompatibledubpackage --version=1.0.2 --skip-registry=
 if [ $? -eq 124 ]; then
     die 'Fetching from responsive registry should not time-out.'
 fi
-dub remove gitcompatibledubpackage --non-interactive --version=1.0.2
+"$DUB" remove gitcompatibledubpackage --non-interactive --version=1.0.2

Fixed:

diff --git test/fetchzip.sh test/fetchzip.sh
index 4bbac58..6839a44 100755
--- test/fetchzip.sh
+++ test/fetchzip.sh
@@ -32,8 +32,8 @@ if ! zipCount=$(grep -Fc 'Failed to extract zip archive' <<<"$zipOut") || [ "$zi
 elif [ $rc -eq 124 ]; then
     die 'DUB timed out unexpectedly.'
 fi
-    die 'DUB should not have installed a broken package.'
 if "$DUB" remove gitcompatibledubpackage --non-interactive --version=* 2>/dev/null; then
+    die 'DUB should not have installed a broken package.'
 fi
 
 echo "HTTP status errors on downloads should be retried - gitcompatibledubpackage (1.0.2)"
@@ -47,8 +47,8 @@ if ! retryCount=$(echo "$retryOut" | grep -Fc 'Bad Gateway') || [ "$retryCount"
 elif [ $rc -eq 124 ]; then
     die 'DUB timed out unexpectedly.'
 fi
-    die 'DUB should not have installed a package.'
 if "$DUB" remove gitcompatibledubpackage --non-interactive --version=* 2>/dev/null; then
+    die 'DUB should not have installed a package.'
 fi
 
 echo "HTTP status errors on downloads should retry with fallback mirror - gitcompatibledubpackage (1.0.2)"

And rebased.

@PetarKirov PetarKirov force-pushed the dub-binary branch 16 times, most recently from 6eaa1c3 to e02f89d Compare August 8, 2019 15:06
@PetarKirov
Copy link
Member

I have resolved the merge conflicts (more like redid the PR from the beginning 😄 ) and I will push an update soon

@PetarKirov PetarKirov changed the title [WIP] Use $DUB instead of dub and ensure that global DUB isn't available in the testsuite Various improvements to the testing shell scripts Nov 27, 2020
@PetarKirov PetarKirov force-pushed the dub-binary branch 2 times, most recently from 0babc44 to 8367462 Compare November 27, 2020 13:38
@PetarKirov PetarKirov force-pushed the dub-binary branch 2 times, most recently from 36641fe to 36ad3b2 Compare November 27, 2020 14:01
* Add comments to the script
* Print information on the detected HOST_DUB and DC env variables
Copy link
Member

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks about right, just a few nits / questions

}

if [ "$COVERAGE" = true ]; then
if [ "${COVERAGE:-}" = true ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the quotes then, do you ?

## Here the `COVERAGE` variable is abused for this purpose,
## as it's only defined once in the whole Travis matrix
if [ "$COVERAGE" = true ]; then
if [ "${COVERAGE:-}" = true ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

Comment on lines +13 to +15
export HOST_DUB
HOST_DUB="$(command -v dub)" ||
{ echo >&2 "[ERROR] 'dub' should be available"; exit 1; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't those lines be merged ?

hash -p /dev/null/dub dub

# 2/3 Verify that both 'dub' and "$DUB" are not available:
dub > /dev/null 2>&1 || command -v "${DUB:-}" >/dev/null && \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use dub -h maybe ?


# 2/3 Verify that both 'dub' and "$DUB" are not available:
dub > /dev/null 2>&1 || command -v "${DUB:-}" >/dev/null && \
{ echo >&2 "[ERROR] 'dub' shouldn't be available"; exit 1; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't be available "anymore" ?
Otherwise the error message might be confusing.

HOST_DUB="$(command -v dub)" ||
{ echo >&2 "[ERROR] 'dub' should be available"; exit 1; }
unset DUB
hash -p /dev/null/dub dub
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've no idea what this does. Do you have a link for me ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants