[Openvpn-devel] t_net.sh: drop hard dependency on t_client.rc

Message ID 20200717160231.12042-1-a@unstable.cc
State Changes Requested
Delegated to: Gert Doering
Headers show
Series
  • [Openvpn-devel] t_net.sh: drop hard dependency on t_client.rc
Related show

Commit Message

Antonio Quartulli July 17, 2020, 4:02 p.m.
Right now t_net.sh depends on t_client.rc in order to source the
RUN_SUDO variable only.
However, t_client.rc is something that a few people only have configured
and thus this would result in t_net.sh almost never executed even if it
just could.

Drop dependency on t_client.rc by falling back to RUN_SUDO=sudo when the
file is missing.

The assignment is made as conditional so that a user can still override
RUN_SUDO by speciying an alternate string on the command line.

While at it, reword the error message to better match the current logic
flow.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 tests/t_net.sh | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Gert Doering July 21, 2020, 4:39 p.m. | #1
Hi,

On Fri, Jul 17, 2020 at 06:02:31PM +0200, Antonio Quartulli wrote:
> Right now t_net.sh depends on t_client.rc in order to source the
> RUN_SUDO variable only.

I was about to merge this ("nice and easy") but I think it's just
complicated.

> diff --git a/tests/t_net.sh b/tests/t_net.sh
> index c67c3df2..63732db9 100755
> --- a/tests/t_net.sh
> +++ b/tests/t_net.sh
> @@ -77,9 +77,7 @@ if [ -r "${top_builddir}"/t_client.rc ]; then
>  elif [ -r "${srcdir}"/t_client.rc ]; then
>      . "${srcdir}"/t_client.rc
>  else
> -    echo "$0: cannot find 't_client.rc' in build dir ('${top_builddir}')" >&2
> -    echo "$0: or source directory ('${srcdir}'). SKIPPING TEST." >&2
> -    exit 77
> +    RUN_SUDO="${RUN_SUDO:-sudo}"
>  fi

Leave off the whole else clause.

>  if [ ! -x "$openvpn" ]; then
> @@ -117,8 +115,7 @@ else
>  
>      if [ -z "$RUN_SUDO" ]
>      then
> -        echo "$0: this test must run be as root, or RUN_SUDO=... " >&2
> -        echo "      must be set correctly in 't_client.rc'. SKIP." >&2
> +        echo "$0: using t_client.rc, but RUN_SUDO=... is not defined correctly. SKIP. " >&2
>          exit 77

In here, print & set

      if [ -z "$RUN_SUDO" ]
      then
 +        echo "$0: no RUN_SUDO=... in t_client.rc or environment, defaulting to 'sudo'." >&2
 +        echo "      if that does not work, set RUN_SUDO= correctly for your system." >&2
 +        RUN_SUDO=sudo"
      fi

done - less code, message conveyed if needed.
  
gert
Antonio Quartulli July 21, 2020, 7:04 p.m. | #2
Hi,

On 21/07/2020 18:39, Gert Doering wrote:
> In here, print & set
> 
>       if [ -z "$RUN_SUDO" ]
>       then
>  +        echo "$0: no RUN_SUDO=... in t_client.rc or environment, defaulting to 'sudo'." >&2
>  +        echo "      if that does not work, set RUN_SUDO= correctly for your system." >&2
>  +        RUN_SUDO=sudo"
>       fi
> 
> done - less code, message conveyed if needed.
>   


hmhmhmh makes sense. v2 incoming!

Patch

diff --git a/tests/t_net.sh b/tests/t_net.sh
index c67c3df2..63732db9 100755
--- a/tests/t_net.sh
+++ b/tests/t_net.sh
@@ -77,9 +77,7 @@  if [ -r "${top_builddir}"/t_client.rc ]; then
 elif [ -r "${srcdir}"/t_client.rc ]; then
     . "${srcdir}"/t_client.rc
 else
-    echo "$0: cannot find 't_client.rc' in build dir ('${top_builddir}')" >&2
-    echo "$0: or source directory ('${srcdir}'). SKIPPING TEST." >&2
-    exit 77
+    RUN_SUDO="${RUN_SUDO:-sudo}"
 fi
 
 if [ ! -x "$openvpn" ]; then
@@ -117,8 +115,7 @@  else
 
     if [ -z "$RUN_SUDO" ]
     then
-        echo "$0: this test must run be as root, or RUN_SUDO=... " >&2
-        echo "      must be set correctly in 't_client.rc'. SKIP." >&2
+        echo "$0: using t_client.rc, but RUN_SUDO=... is not defined correctly. SKIP. " >&2
         exit 77
     else
         # check that we can run the unit-test binary with sudo