Merge ~jslarraz/review-tools:remove-duplicate-tests-in-test-sh into review-tools:master

[Jorge Sancho Larraz]

tests/test.sh: only run tests once per snap. A special environment variable `RT_FUNC_TEST` is used to instruct snap-review to prints the report in the three different formats ('', '--skd', '--json')

[Alex Murray (alexmurray)]

LGTM except minor change needed.

review: Needs Fixing

[Jorge Sancho Larraz (jslarraz)]

Comment added in https://git.launchpad.net/~jslarraz/review-tools/commit/?id=6e55ec02ed5fe7bbbe4f39be217e56a67105db0d

[Alex Murray (alexmurray)]

LGTM - thanks @jslarraz

review: Approve

diff --git a/bin/click-review b/bin/click-review
deleted file mode 120000
index 768dbca..0000000
--- a/bin/click-review
+++ /dev/null
@@ -1 +0,0 @@
-snap-review
\ No newline at end of file
diff --git a/bin/snap-review b/bin/snap-review
index 6a70d1c..d457965 100755
--- a/bin/snap-review
+++ b/bin/snap-review
@@ -193,20 +193,41 @@ def main():
                 "Could not write state output: %s" % e,
             )
 
+    # tests/test.sh called snap-review three times for each snap to generate
+    # the output in different formats ("", "--sdk", "--json"). RT_FUNC_TEST env
+    # variable is used to generate the expected output (tests/test.sh.expected)
+    # only running the review once
+    if os.getenv("RT_FUNC_TEST"):
+        allow_classic = " --allow-classic" if args.allow_classic else ""
+        rt_extras = " RT_EXTRAS_PATH with" if os.getenv("RT_EXTRAS_PATH") else ""
+        prefix = allow_classic + rt_extras
+
+        print("=%s  %s =" % (prefix, os.path.basename(args.filename)))
+        report.show_results("console")
+        print("")
+
+        print("=%s --sdk %s =" % (prefix, os.path.basename(args.filename)))
+        report.show_results("sdk")
+        print("")
+
+        print("=%s --json %s =" % (prefix, os.path.basename(args.filename)))
+        report.show_results("json")
+        print("")
+
+        sys.exit(0)
+
     # Show the results of the report
     if args.json:
-        report_type = "json"
+        report.show_results("json")
     elif args.sdk:
-        report_type = "sdk"
+        report.show_results("sdk")
         if report.errors or report.warnings:
             # TODO: harmonize return codes
             rc = 1
     else:
-        report_type = "console"
-
-    # Show results updates of report.errors and report.warnings
-    report.show_results(report_type)
+        report.show_results("console")
 
+    # Return codes
     if rc == 1:
         # always exit(1) if there are errors
         pass
diff --git a/tests/test.sh b/tests/test.sh
index 947b773..29dd02c 100755
--- a/tests/test.sh
+++ b/tests/test.sh
@@ -15,10 +15,9 @@ if [ "$1" = "help" ] || [ "$1" = "--help" ] || [ "$1" = "-h" ]; then
     exit
 fi
 
-# TODO: figure out a different way to determine if performing system tests
-testtype="click-review"
+review_executable="bin/snap-review"
 if [ "$1" = "system" ]; then
-   testtype="snap-review"
+   review_executable="review-tools.snap-review"
    # shellcheck disable=SC2230
    if ! which review-tools.snap-review >/dev/null 2>&1 ; then
        echo "Could not find snap-review. Is the review-tools snap installed?"
@@ -38,60 +37,18 @@ tmpjson=$(mktemp)
 
 for i in ./tests/*.snap ; do
     echo "$i" | grep -q "/test-resquash_" && continue  # test these elsewhere
-    for j in "" "--sdk" "--json" ; do
-        snap=$(basename "$i")
-        echo "= $j $snap ="
-        if [ "$testtype" = "snap-review" ]; then
-            review-tools.snap-review $j "$i" 2>&1 | sed -e 's#./tests/##g' -e 's/"text": "SKIPPED (could not import apt_pkg)"/"text": "OK"/' | tee "$tmpjson"
-        else
-            PYTHONPATH=./ ./bin/click-review $j "$i" 2>&1 | sed -e 's#./tests/##g' | tee "$tmpjson"
-        fi
-
-        if [ "$j" = "--json" ]; then
-            jq '.' "$tmpjson" >/dev/null || {
-                echo "'jq . $tmpjson' failed"
-                cat "$tmpjson"
-                exit 1
-            }
-        fi
-        echo
-    done
+    RT_FUNC_TEST=1 PYTHONPATH=. "$review_executable" "$i" 2>&1 | sed -e 's#./tests/##g' | tee "$tmpjson"
 done | tee "$tmp"
 
 for i in ./tests/test-classic*.snap ; do
-    for j in "" "--sdk" "--json" ; do
-        snap=$(basename "$i")
-        echo "= --allow-classic $j $snap ="
-        if [ "$testtype" = "snap-review" ]; then
-            review-tools.snap-review --allow-classic $j "$i" 2>&1 | sed -e 's#./tests/##g' -e 's/"text": "SKIPPED (could not import apt_pkg)"/"text": "OK"/' | tee "$tmpjson"
-        else
-            PYTHONPATH=./ ./bin/click-review --allow-classic $j "$i" 2>&1 | sed -e 's#./tests/##g' | tee "$tmpjson"
-        fi
-
-        if [ "$j" = "--json" ]; then
-            jq '.' "$tmpjson" >/dev/null || {
-                echo "'jq . $tmpjson' failed"
-                exit 1
-            }
-        fi
-        echo
-    done
+    RT_FUNC_TEST=1 PYTHONPATH=. "$review_executable" --allow-classic "$i" 2>&1 | sed -e 's#./tests/##g' | tee "$tmpjson"
 done | tee -a "$tmp"
 
 # test-resquash_0.snap takes forever. Just test it with --json
 for i in ./tests/test-resquash_*.snap ; do
     snap=$(basename "$i")
     echo "= --json $snap ="
-    if [ "$testtype" = "snap-review" ]; then
-        review-tools.snap-review --json "$i" 2>&1 | sed -e 's#./tests/##g' -e 's/"text": "SKIPPED (could not import apt_pkg)"/"text": "OK"/' > "$tmpjson"
-    else
-        PYTHONPATH=./ ./bin/click-review --json "$i" 2>&1 | sed -e 's#./tests/##g' > "$tmpjson"
-    fi
-
-    jq '.' "$tmpjson" >/dev/null || {
-        echo "'jq . $tmpjson' failed"
-        exit 1
-    }
+    PYTHONPATH=. "$review_executable" --json "$i" 2>&1 | sed -e 's#./tests/##g' > "$tmpjson"
 
     # now show all the output that with the deleted error text
     jq 'del(."snap.v2_security"."error"."security-snap-v2:squashfs_files"."text")' "$tmpjson"
@@ -101,26 +58,9 @@ for i in ./tests/test-resquash_*.snap ; do
 done | tee -a "$tmp"
 
 # test RT_EXTRAS_PATH
-testsnap="./tests/hello-world_25.snap"
 rtpath="./tests/review-tools-extras"
-for j in "" "--sdk" "--json" ; do
-    snap=$(basename "$testsnap")
-    echo "= RT_EXTRAS_PATH with $j $testsnap ="
-    if [ "$testtype" = "snap-review" ]; then
-        RT_EXTRAS_PATH="$rtpath" review-tools.snap-review $j "$testsnap" 2>&1 | sed -e 's#./tests/##g' -e 's/"text": "SKIPPED (could not import apt_pkg)"/"text": "OK"/' | tee "$tmpjson"
-    else
-        RT_EXTRAS_PATH="$rtpath" PYTHONPATH=./ ./bin/click-review $j "$testsnap" 2>&1 | sed -e 's#./tests/##g' | tee "$tmpjson"
-    fi
-
-    if [ "$j" = "--json" ]; then
-        jq '.' "$tmpjson" >/dev/null || {
-            echo "'jq . $tmpjson' failed"
-            cat "$tmpjson"
-            exit 1
-        }
-    fi
-    echo
-done | tee -a "$tmp"
+testsnap="./tests/hello-world_25.snap"
+RT_FUNC_TEST=1 PYTHONPATH=. RT_EXTRAS_PATH="$rtpath" "$review_executable" "$testsnap" 2>&1 | sed -e 's#./tests/##g' | tee -a "$tmp"
 
 echo
 echo "Checking for differences in output..."
diff --git a/tests/test.sh.expected b/tests/test.sh.expected
index e74cc2c..4a8281f 100644
--- a/tests/test.sh.expected
+++ b/tests/test.sh.expected
@@ -116895,7 +116895,7 @@ unusual mode 'rwxrwxrwt' for entry './file/1777-daemon:daemon'
 unusual mode 'rwxrwxrwt' for entry './file/1777-daemon:root'
 unusual mode 'rwxrwxrwt' for entry './file/1777-root:daemon'
 unusual mode 'rwxrwxrwt' for entry './file/1777-root:root'
-= RT_EXTRAS_PATH with  ./tests/hello-world_25.snap =
+= RT_EXTRAS_PATH with  hello-world_25.snap =
 Errors
 ------
  - extras-tests-v2:error
@@ -116906,7 +116906,7 @@ Warnings
 	test warn
 hello-world_25.snap: FAIL
 
-= RT_EXTRAS_PATH with --sdk ./tests/hello-world_25.snap =
+= RT_EXTRAS_PATH with --sdk hello-world_25.snap =
 = snap.v2_declaration =
 {
   "error": {},
@@ -117188,7 +117188,7 @@ hello-world_25.snap: FAIL
   "warn": {}
 }
 
-= RT_EXTRAS_PATH with --json ./tests/hello-world_25.snap =
+= RT_EXTRAS_PATH with --json hello-world_25.snap =
 {
   "snap.v2_declaration": {
     "error": {},