Fix & unit test awscli version comparison function

hooks/environment

@@ -1,5 +1,4 @@
 #!/bin/bash
-
 set -eu -o pipefail
 
 # Reads either a value or a list from plugin config
@@ -22,21 +21,30 @@ function plugin_read_list() {
 
 # Check a provided aws-cli version is greater or equal than the current
 function aws_version_ge() {
-  local version; version="$(aws --version 2>&1 | awk -F'[/ ]' '{print $2}')"
-  IFS="." read -r -a wanted <<< "$1"
-  IFS="." read -r -a current <<< "$version"
+  local current; current="$(aws --version 2>&1 | awk -F'[/ ]' '{print $2}')"
+  local wanted="$1"
+  version_a_gte_b "$current" "$wanted"
+}
 
-  if [[ ! ${#current[@]} -eq 3 ]] ; then
-    echo "Expected $version to be in the form x.y.z" >&2
+# Whether version A >= B, for versions in the form of major.minor.patch
+# Returns success (0) if true, and error (1) if false.
+function version_a_gte_b() {
+  IFS="." read -r -a a <<< "$1"
+  if [[ ! ${#a[@]} -eq 3 ]] ; then
+    echo "Expected $1 to be in the form x.y.z" >&2
     exit 1
   fi
-
-  [[ ${current[0]} -gt ${wanted[0]} ]] && return 0
-  [[ ${current[1]} -gt ${wanted[1]} ]] && return 0
-  [[ ${current[2]} -ge ${wanted[2]} ]] && return 0
-
-  echo "$version isn't ge $1" >&2
-  return 1
+  IFS="." read -r -a b <<< "$2"
+  if [[ ! ${#b[@]} -eq 3 ]] ; then
+    echo "Expected $2 to be in the form x.y.z" >&2
+    exit 1
+  fi
+  [[ ${a[0]} -lt ${b[0]} ]] && return 1 # major less
+  [[ ${a[0]} -gt ${b[0]} ]] && return 0 # major more
+  [[ ${a[1]} -lt ${b[1]} ]] && return 1 # major same, minor less
+  [[ ${a[1]} -gt ${b[1]} ]] && return 0 # major same, minor more
+  [[ ${a[2]} -lt ${b[2]} ]] && return 1 # major same, minor same, patch less
+  return 0                              # major same, minor same, patch same or more
 }
 
 # Retries a command on failure.
@@ -65,20 +73,20 @@ if [[ -z "${AWS_DEFAULT_REGION:-}" ]] ; then
   export AWS_DEFAULT_REGION=us-east-1
 fi
 
-# If not specified, auto-detect if we can support no-include-email
-if [[ -z "${BUILDKITE_PLUGIN_ECR_NO_INCLUDE_EMAIL:-}" ]] ; then
-  if aws_version_ge "1.11.91" ; then
-    BUILDKITE_PLUGIN_ECR_NO_INCLUDE_EMAIL="true"
-  else
-    BUILDKITE_PLUGIN_ECR_NO_INCLUDE_EMAIL="false"
-  fi
-fi
-
 # For logging into the current AWS account’s registry
 if [[ "${BUILDKITE_PLUGIN_ECR_LOGIN:-}" =~ ^(true|1)$ ]] ; then
   mapfile -t registry_ids <<< "$(plugin_read_list ACCOUNT_IDS | tr "," "\n")"
   login_args=()
 
+  # If not specified, auto-detect if we can support no-include-email
+  if [[ -z "${BUILDKITE_PLUGIN_ECR_NO_INCLUDE_EMAIL:-}" ]] ; then
+    if aws_version_ge "1.11.91" ; then
+      BUILDKITE_PLUGIN_ECR_NO_INCLUDE_EMAIL="true"
+    else
+      BUILDKITE_PLUGIN_ECR_NO_INCLUDE_EMAIL="false"
+    fi
+  fi
+
   if [[ $BUILDKITE_PLUGIN_ECR_NO_INCLUDE_EMAIL =~ (true|on|1) ]] ; then
     login_args+=("--no-include-email")
   fi

tests/version.bats

@@ -0,0 +1,44 @@
+#!/usr/bin/env bats
+load '/usr/local/lib/bats/load.bash'
+load "$PWD/hooks/environment"
+
+@test "version_a_gte_b: basic: major less; false" {
+  run version_a_gte_b "1.3.3" "3.2.1"
+  assert_failure
+}
+@test "version_a_gte_b: basic: major more; true" {
+  run version_a_gte_b "3.2.1" "1.3.2"
+  assert_success
+}
+@test "version_a_gte_b: basic: major same, minor less; false" {
+  run version_a_gte_b "3.2.1" "3.3.0"
+  assert_failure
+}
+@test "version_a_gte_b: basic: major same, minor more; true" {
+  run version_a_gte_b "3.2.1" "3.1.2"
+  assert_success
+}
+@test "version_a_gte_b: basic: major same, minor same, patch same; true" {
+  run version_a_gte_b "1.1.1" "1.1.1"
+  assert_success
+}
+@test "version_a_gte_b: basic: major same, minor same, patch more; true" {
+  run version_a_gte_b "1.1.2" "1.1.1"
+  assert_success
+}
+@test "version_a_gte_b: basic: major same, minor same, patch less; false" {
+  run version_a_gte_b "1.1.1" "1.1.2"
+  assert_failure
+}
+@test "version_a_gte_b: specific: 1.11.40 >= 1.11.91; false" {
+  run version_a_gte_b "1.11.40" "1.11.91"
+  assert_failure
+}
+@test "version_a_gte_b: specific: 2.0.2 >= 1.11.91; true" {
+  run version_a_gte_b "2.0.2" "1.11.91"
+  assert_success
+}
+@test "version_a_gte_b: specific: 2.0.2 >= 2.0.0; true" {
+  run version_a_gte_b "2.0.2" "2.0.0"
+  assert_success
+}