Ci check deleted.txt when translators are deleted (zotero#1238)

* ci: check translators for reused IDs * ci: check for deleted.txt consistency * Add multistr option to jshintrc * .ci/checkDeletedTxt.sh: Warn/exit if master not available Actually, this script will either be used in a local feature branch and then we can assume that master branch is also available, or in the Travis check of the official repo, where in each PR the master branch is available. In all other cases we skip now the test and show just a warning that we are doing that. * Add option to call checkTranslator.sh with label only
adam3smith · Jan 29, 2017 · bb460b053e6625f7dbf539dc11309d3af2cc1860 · bb460b0
1 parent d8a5aa8
commit bb460b053e6625f7dbf539dc11309d3af2cc1860
Unified Split

Showing with 112 additions and 25 deletions.

+14 −6 .ci/README.md

+46 −0 .ci/checkDeletedTxt.sh

+28 −18 .ci/checkTranslator.sh

+21 −0 .ci/helper.sh

+2 −1 .ci/jshintrc

+1 −0 .travis.yml
diff --git a/.ci/README.md b/.ci/README.md
@@ -22,20 +22,28 @@ You can use the npm bindings to run tests on all translators:
 (faster, because up to 8 cores are used, such that output will not stay in alphabetical order)
-  
-## Using the script directly
+  
+## Using the scripts directly
 You can use the bash script `checkTranslator.sh` directly to
 run tests on a single translator, e.g.
 ```
+  
+# in .ci directory:
 ./checkTranslator.sh ../Amazon.js
+  
+
+  
+# in translators directory:
+  
+.ci/checkTranslator.sh Amazon.js
 ```
-  
-Moreover, you can skip the warnings by setting the environment
-  
-variable `SKIP_WARN` to `true`:
+  
+Moreover, you can skip the warnings by using the `--skip-warn`
+  
+option and reference the translators just by their label, e.g.
 ```
-  
-export SKIP_WARN=true
-  
-./checkTranslator.sh ../Amazon.com
+  
+./checkTranslator.sh --skip-warn Amazon
 ```
-  
-To switch back just set the variable to the empty string.
+  
+You can also check the `deleted.txt` when deleting translators by
+  
+```
+  
+./checkDeletedTxt.sh
+  
+```
+  
+(This does a comparison with the master branch, which must be there but
+  
+currently not checked out. Otherwise the test is skipped.)
diff --git a/.ci/checkDeletedTxt.sh b/.ci/checkDeletedTxt.sh
@@ -0,0 +1,46 @@
+  
+#!/bin/bash
+  
+
+  
+SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )
+  
+source "$SCRIPT_DIR/helper.sh"
+  
+cd "$SCRIPT_DIR"
+  
+
+  
+MASTER="master"
+  
+BRANCH=$(git rev-parse --abbrev-ref HEAD)
+  
+
+  
+if [[ "$BRANCH" = "$MASTER" ]];then
+  
+    echo "${color_warn}skip${color_reset} - Can only check deleted.txt when not on '$MASTER' branch"
+  
+    exit 0
+  
+fi
+  
+
+  
+if ! git branch |grep -q "$MASTER"; then
+  
+    echo "${color_warn}skip${color_reset} - Can only check deleted.txt when '$MASTER' branch is also available for comparison"
+  
+    exit 0
+  
+fi
+  
+
+  
+main() {
+  
+    local IFS=$'\n'
+  
+    declare -a deletions=()
+  
+    local failed=0
+  
+    # Find all deleted js files
+  
+    deletions+=($(git diff-index --diff-filter=D --name-only $MASTER|grep -v '\.ci'|grep 'js$'))
+  
+    if (( ${#deletions[@]} > 0 ));then
+  
+        for f in "${deletions[@]}";do
+  
+            local id=$(git show $MASTER:"$f"|grepTranslatorId)
+  
+            if ! grep -qF "$id" '../deleted.txt';then
+  
+                echo "${color_notok}not ok${color_reset} - $id ($f) should be added to deleted.txt"
+  
+                (( failed += 1))
+  
+            fi
+  
+        done
+  
+        curVersion=$(head -n1 "../deleted.txt"|grep -o '[0-9]\+')
+  
+        origVersion=$(git show "$MASTER:deleted.txt"|head -n1|grep -o '[0-9]\+')
+  
+        if (( curVersion <= origVersion ));then
+  
+            echo "${color_notok}not ok${color_reset} - version in deleted.txt needs to be increased"
+  
+            (( failed += 1))
+  
+        fi
+  
+    fi
+  
+    if [[ "$failed" = 0 ]];then
+  
+        echo "${color_ok}ok${color_reset} - deleted.txt"
+  
+    fi
+  
+    exit $failed
+  
+}
+  
+main
diff --git a/.ci/checkTranslator.sh b/.ci/checkTranslator.sh
@@ -1,5 +1,9 @@
 #!/bin/bash
+  
+# Load colors and such
+  
+SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )
+  
+source "$SCRIPT_DIR/helper.sh"
+  
+
 # Global variable holding the currently checked file
 TRANSLATOR=
 TRANSLATOR_BASENAME=
@@ -8,6 +12,7 @@ TRANSLATOR_BASENAME=
 declare -a ERROR_CHECKS=(
    "notOneTranslatorID"
    "nonUniqueTranslatorID"
+  
+    "reusingDeletedID"
    "executableFile"
    "invalidJSON"
    "withCarriageReturn"
@@ -40,19 +45,27 @@ notOneTranslatorID () {
 nonUniqueTranslatorID () {
    local dir id duplicateIds
    dir=$(dirname "$TRANSLATOR")
-  
-    id=$(grep -r '"translatorID"' "$TRANSLATOR" \
-  
-        | sed -e 's/[" ,]//g' -e 's/^.*://g')
+  
+    id=$(grepTranslatorId "$TRANSLATOR")
    if [[ -n "$id" ]];then
-  
-      duplicateIds=$(grep -r '"translatorID"' "$dir"/*.js \
-  
-              | sed -e 's/[" ,]//g' -e 's/^.*://g' \
-  
-              | sort | uniq -d)
+  
+      duplicateIds=$(grepTranslatorId "$dir"/*.js | sort | uniq -d)
      if [[ $duplicateIds = *"$id"* ]];then
          err "Non unique ID $id"
          return 1
      fi
    fi
 }
+  
+# IDs must not reuse deleted IDs
+  
+reusingDeletedID () {
+  
+    local dir id duplicateIds
+  
+    dir=$(dirname "$TRANSLATOR")
+  
+    id=$(grepTranslatorId "$TRANSLATOR")
+  
+    if grep -qF "$id" "$dir/deleted.txt";then
+  
+        err "Is reusing an earlier deleted ID"
+  
+        return 1
+  
+    fi
+  
+}
+  
+
 # Translators should not be executable
 executableFile () {
    if [[ -x "$TRANSLATOR" ]];then
@@ -97,7 +110,7 @@ problematicJS () {
    jshint_error=$(sed '1,/^}/ s/.*//' "$TRANSLATOR" \
        | sed 's,/\*\* BEGIN TEST,\n\0,' \
        | sed '/BEGIN TEST/,$ d' \
-  
-        | jshint --config=jshintrc --reporter=unix -)
+  
+        | jshint --config="$SCRIPT_DIR"/jshintrc --reporter=unix -)
    if (( $? > 0 ));then
        warn "JSHint shows issues with this code"
        warn "$jshint_error"
@@ -109,21 +122,14 @@ problematicJS () {
 # Helpers and main
 #-----------------------------------------------------------------------
-  
-if [[ -t 1 && "$(tput colors)" -gt 0 ]]; then
-  
-    color_ok=$'\e[32;1m'
-  
-    color_notok=$'\e[31;1m'
-  
-    color_warn=$'\e[33m'
-  
-    color_err=$'\e[31m'
-  
-    color_reset=$'\e[0m'
-  
-fi
 err () { echo -e "$*" | sed "s/^/# ${color_err}ERROR:${color_reset} $TRANSLATOR_BASENAME :/" >&2; }
 warn () { echo -e "$*" | sed "s/^/# ${color_warn}WARN: ${color_reset} $TRANSLATOR_BASENAME :/" >&2; }
-  
-usage () { (( $# > 0 )) && err "$*"; err "Usage: $0 <translator.js>"; }
+  
+usage () { (( $# > 0 )) && err "$*"; err "Usage: $0 [--skip-warn] <translator.js>"; }
 main() {
    # Add './node_modules/.bin to PATH for jsonlint
-  
-    PATH=$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )/node_modules/.bin:"$PATH"
+  
+    PATH=$SCRIPT_DIR/node_modules/.bin:"$PATH"
    if [[ "$1" = "--skip-warn" ]];then
        SKIP_WARN=true
@@ -132,7 +138,11 @@ main() {
    TRANSLATOR="$1"
    TRANSLATOR_BASENAME="$(basename "$TRANSLATOR")"
    [[ -z $TRANSLATOR ]]   && { usage; exit 1; }
-  
-    [[ ! -e $TRANSLATOR ]] && { usage "File/Directory not found."; exit 2; }
+  
+    if [[ ! -e $TRANSLATOR ]];then
+  
+        # Construct filename and path if $TRANSLATOR is only used as label
+  
+        TRANSLATOR="$(dirname $SCRIPT_DIR)/${TRANSLATOR}.js"
+  
+    fi
+  
+    [[ ! -e $TRANSLATOR ]] && { usage "File/Directory not found.\n$TRANSLATOR"; exit 2; }
    [[ -d $TRANSLATOR ]]   && { usage "Must be a file not a directory."; exit 3; }
    declare -a errors=() warnings=()
@@ -148,10 +158,10 @@ main() {
        fi
    else
        if (( ${#warnings[@]} == 0 ));then
-  
-            echo "${color_notok}not ok${color_reset} - $TRANSLATOR (Failed checks: ${errors[*]})"
+  
+            echo "${color_notok}not ok${color_reset} - $TRANSLATOR (Errors: ${errors[*]})"
            exit 1
        else
-  
-            echo "${color_notok}not ok${color_reset} - $TRANSLATOR (Failed checks: ${errors[*]}; Warnings: ${warnings[*]})"
+  
+            echo "${color_notok}not ok${color_reset} - $TRANSLATOR (Errors: ${errors[*]}; Warnings: ${warnings[*]})"
            exit 1
        fi
    fi

diff --git a/.ci/helper.sh b/.ci/helper.sh
@@ -0,0 +1,21 @@
+  
+#!/bin/bash
+  
+
+  
+if [[ -t 1 && "$(tput colors)" -gt 0 ]]; then
+  
+    export color_ok=$'\e[32;1m'
+  
+    export color_notok=$'\e[31;1m'
+  
+    export color_warn=$'\e[33m'
+  
+    export color_err=$'\e[31m'
+  
+    export color_reset=$'\e[0m'
+  
+fi
+  
+
+  
+grepTranslatorId () {
+  
+    if [[ -n "$1" ]];then
+  
+        grep -r '"translatorID"' "$@" | sed -e 's/[" ,]//g' -e 's/^.*://g'
+  
+    else
+  
+        while read line;do
+  
+            echo "$line"|grep '"translatorID"' | sed -e 's/[" ,]//g' -e 's/^.*://g'
+  
+        done
+  
+    fi
+  
+}
+  
+
+  
+# vim: ft=sh
diff --git a/.ci/jshintrc b/.ci/jshintrc
@@ -1,3 +1,4 @@
 {
-  
-  "laxbreak": true
+  
+  "laxbreak": true,
+  
+  "multistr": true
 }
diff --git a/.travis.yml b/.travis.yml
@@ -3,3 +3,4 @@ node_js:
  - "6"
 script:
  - cd .ci && npm install && npm test
+  
+  - ./checkDeletedTxt.sh