tests: remove quoting from [[ ]] when globs #3264

Merged
merged 3 commits into from May 4, 2017

Conversation

Projects
None yet
4 participants
Contributor

zyga commented May 3, 2017

It seems that shell has a peculiar quoting rules when [[ ]] expressions
are used. [[ "foo" == f* ]] returns 0 while [[ "foo" == "f*" ]] returns 1.
The glob magic is on only if the expression on the right hand side is
unquoted.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

tests: remove quoting from [[ ]] when globs
It seems that shell has a peculiar quoting rules when [[ ]] expressions
are used. [[ "foo" == f* ]] returns 0 while [[ "foo" == "f*" ]] returns
1. The glob magic is on only if the expression on the right hand side is
unquoted.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
Contributor

morphis commented May 3, 2017

LGTM, @zyga btw. are we using signed-off-by for snapd? I never used it so far and we should make it either a common thing everybody does or drop it.

chipaca approved these changes May 3, 2017

Gah! I knew this, and missed in the last review. Sorry.

Contributor

ogra1 commented May 3, 2017

double brackets are a bash only feature, please do not use it if you can avoid it or make sure that either /bin/sh points to /bin/bash in your environment or your shebang is explicitly set to #! /bin/bash (to be more portable we should really make sure to only use POSIX shell unless it is unavoidable)
see https://wiki.ubuntu.com/DashAsBinSh#A.5B.5B

Member

chipaca commented May 3, 2017

@ogra1 this is in a spread task.yaml, which is bash and not sh

Contributor

ogra1 commented May 3, 2017

@chipaca well, as long as we are sure this never runs in other distros or setups ... it doesnt do any harm to use [ ] over [[ ]] though (and makes no difference in execution apart from being portable)

Member

chipaca commented May 3, 2017

@ogra1 except that the reason it's using [[ instead of [ is to get the pattern matching which [ does not provide :-)
And sure, it could be done with grep instead.

Contributor

ogra1 commented May 3, 2017

case "$SPREAD_SYSTEM" in
    debian-*)
        exit 0
    ;;
esac

... about ten times as fast and more efficient ... (and portable as well)

Member

chipaca commented May 3, 2017

After re-running spread a number of times, I need to ask: are you sure you want to exit early from the test when the system isn't debian? I'm suspecting you meant the opposite.

Contributor

zyga commented May 4, 2017

@morphis can you have a look at @chipaca's question please?

Contributor

morphis commented May 4, 2017

After re-running spread a number of times, I need to ask: are you sure you want to exit early from the test when the system isn't debian? I'm suspecting you meant the opposite.

The test should be $SPREAD_SYSTEM = debian-* instead of $SPREAD_SYSTEM != debian-*

This should solve the still failing CI.

@zyga Can you change that?

Member

chipaca commented May 4, 2017

@morphis wouldn't it make sense to make it be != ubuntu-*?

Contributor

morphis commented May 4, 2017

@morphis wouldn't it make sense to make it be != ubuntu-*?

We could but I will propose a change before we land any other SPREAD_SYSTEM which will make this check superfluous so lets fix this the easy way for now.

tests: fix inverted test logic
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@chipaca chipaca merged commit cc04482 into snapcore:master May 4, 2017

6 of 7 checks passed

xenial-i386 autopkgtest finished (failure)
Details
artful-amd64 autopkgtest finished (success)
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details

@zyga zyga deleted the zyga:fix/quoting-and-globs branch Aug 22, 2017

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