You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2014/01/17 19:04:42 UTC

git commit: Add a test to encourage taking care with thrift changes.

Updated Branches:
  refs/heads/master 9dd46666c -> 621eb53f4


Add a test to encourage taking care with thrift changes.

We have more to do w.r.t. automating detection and proper handling of schema
changes (specifically in the scheduler).  However, i see this as low-hanging
fruit to at least serve as a reminder.

Reviewed at https://reviews.apache.org/r/16986/


Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/621eb53f
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/621eb53f
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/621eb53f

Branch: refs/heads/master
Commit: 621eb53f40713b484730763860cdfbba0362e120
Parents: 9dd4666
Author: Bill Farner <wf...@apache.org>
Authored: Fri Jan 17 10:04:08 2014 -0800
Committer: Bill Farner <wf...@apache.org>
Committed: Fri Jan 17 10:04:08 2014 -0800

----------------------------------------------------------------------
 build.gradle                                    |  9 ++-
 .../org/apache/aurora/verify_thrift_checksum.sh | 79 ++++++++++++++++++++
 .../org/apache/aurora/gen/api.thrift.md5        |  1 +
 .../apache/aurora/gen/internal_rpc.thrift.md5   |  1 +
 .../org/apache/aurora/gen/storage.thrift.md5    |  1 +
 .../apache/aurora/gen/storage_local.thrift.md5  |  1 +
 .../org/apache/aurora/gen/test.thrift.md5       |  1 +
 .../apache/thermos/thermos_internal.thrift.md5  |  1 +
 8 files changed, 93 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/621eb53f/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index 4a0372f..66f3482 100644
--- a/build.gradle
+++ b/build.gradle
@@ -288,4 +288,11 @@ jacocoTestReport {
   }
 }
 
-test.finalizedBy jacocoTestReport
\ No newline at end of file
+test.finalizedBy jacocoTestReport
+
+task FlagSchemaChanges(type: Test) {
+  exec {
+    executable = 'bash'
+    args = ['src/test/org/apache/aurora/verify_thrift_checksum.sh']
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/621eb53f/src/test/org/apache/aurora/verify_thrift_checksum.sh
----------------------------------------------------------------------
diff --git a/src/test/org/apache/aurora/verify_thrift_checksum.sh b/src/test/org/apache/aurora/verify_thrift_checksum.sh
new file mode 100644
index 0000000..fe8fc58
--- /dev/null
+++ b/src/test/org/apache/aurora/verify_thrift_checksum.sh
@@ -0,0 +1,79 @@
+#!/bin/bash
+set -o nounset
+
+# A test to remind us that we need to exercise care when making API and
+# storage schema changes.
+# There are two operating modes: verify and reset.  Running the script with
+# no arguments will use verify mode.  Reset mode will overwrite all golden
+# files with the current checksum of the source files.
+# In other words: immediately after running in reset mode, the test should
+# pass in verify mode.
+
+reset_mode=false
+while getopts "r" opt
+do
+  case ${opt} in
+    r) reset_mode=true ;;
+    *) exit 1;
+  esac
+done
+
+if $reset_mode
+then
+  echo 'Warning: Operating in reset mode. Rewriting golden checksum files.'
+fi
+
+for file in $(find src/main/thrift -type f -name '*.thrift')
+do
+  # Using python for lack of a better cross-platform checksumming without
+  # adding another build-time dependency.
+  checksum=$(python << EOF
+import hashlib
+m=hashlib.md5()
+m.update(open('$file', 'rb').read())
+print(m.hexdigest())
+EOF)
+  golden_file=src/test/resources/${file#src/main/thrift/}.md5
+  if $reset_mode
+  then
+    mkdir -p $(dirname "$golden_file")
+    echo "$checksum" > "$golden_file"
+  else
+    if [ ! -f "$golden_file" ]
+    then
+      echo "Golden file not found for $file, expected $golden_file"
+      exit 1
+    fi
+    golden_checksum=$(cat "$golden_file")
+    if [ "$checksum" != "$golden_checksum" ]
+    then
+      echo "Golden checksum did not match for $file"
+      echo "Found $checksum, expected $golden_checksum"
+      echo
+      echo $(printf '!%.0s' {1..80})
+      echo 'This means you changed a thrift file.'
+      echo 'Please think carefully before you proceed!'
+      echo
+      echo 'If you are changing an API or a storage schema you may need to '
+      echo 'take additional actions to such as providing a client and/or '
+      echo 'server-side migration strategy.  You may also need to bump the '
+      echo 'released version ID, following the guidelines at http://semver.org'
+      echo
+      echo 'This test is not here to help you make those changes, but to '
+      echo 'remind you to take appropriate follow-up actions relating to your '
+      echo 'schema change.'
+      echo
+      echo 'Once you are confident that you have appropriately supported this '
+      echo 'change in relevant code, you can fix this test by running '
+      echo "sh $0 -r"
+      echo $(printf '!%.0s' {1..80})
+      exit 1
+    fi
+  fi
+done
+
+if ! $reset_mode
+then
+  echo "All checksums match."
+fi
+exit 0

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/621eb53f/src/test/resources/org/apache/aurora/gen/api.thrift.md5
----------------------------------------------------------------------
diff --git a/src/test/resources/org/apache/aurora/gen/api.thrift.md5 b/src/test/resources/org/apache/aurora/gen/api.thrift.md5
new file mode 100644
index 0000000..83fd341
--- /dev/null
+++ b/src/test/resources/org/apache/aurora/gen/api.thrift.md5
@@ -0,0 +1 @@
+03f6423444c34254f26f338a978d0e19

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/621eb53f/src/test/resources/org/apache/aurora/gen/internal_rpc.thrift.md5
----------------------------------------------------------------------
diff --git a/src/test/resources/org/apache/aurora/gen/internal_rpc.thrift.md5 b/src/test/resources/org/apache/aurora/gen/internal_rpc.thrift.md5
new file mode 100644
index 0000000..a324c5a
--- /dev/null
+++ b/src/test/resources/org/apache/aurora/gen/internal_rpc.thrift.md5
@@ -0,0 +1 @@
+1a8adb98193a9a6c47e06d176d9ca691

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/621eb53f/src/test/resources/org/apache/aurora/gen/storage.thrift.md5
----------------------------------------------------------------------
diff --git a/src/test/resources/org/apache/aurora/gen/storage.thrift.md5 b/src/test/resources/org/apache/aurora/gen/storage.thrift.md5
new file mode 100644
index 0000000..f0b16f8
--- /dev/null
+++ b/src/test/resources/org/apache/aurora/gen/storage.thrift.md5
@@ -0,0 +1 @@
+92a4b06480e9d0dfb76ef031240793e0

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/621eb53f/src/test/resources/org/apache/aurora/gen/storage_local.thrift.md5
----------------------------------------------------------------------
diff --git a/src/test/resources/org/apache/aurora/gen/storage_local.thrift.md5 b/src/test/resources/org/apache/aurora/gen/storage_local.thrift.md5
new file mode 100644
index 0000000..5a0a4df
--- /dev/null
+++ b/src/test/resources/org/apache/aurora/gen/storage_local.thrift.md5
@@ -0,0 +1 @@
+d2220b1eee9032472ca7b54163bd66a8

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/621eb53f/src/test/resources/org/apache/aurora/gen/test.thrift.md5
----------------------------------------------------------------------
diff --git a/src/test/resources/org/apache/aurora/gen/test.thrift.md5 b/src/test/resources/org/apache/aurora/gen/test.thrift.md5
new file mode 100644
index 0000000..437a47a
--- /dev/null
+++ b/src/test/resources/org/apache/aurora/gen/test.thrift.md5
@@ -0,0 +1 @@
+61adeee3eda40c06cb78418e22cd23de

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/621eb53f/src/test/resources/org/apache/thermos/thermos_internal.thrift.md5
----------------------------------------------------------------------
diff --git a/src/test/resources/org/apache/thermos/thermos_internal.thrift.md5 b/src/test/resources/org/apache/thermos/thermos_internal.thrift.md5
new file mode 100644
index 0000000..d5a6764
--- /dev/null
+++ b/src/test/resources/org/apache/thermos/thermos_internal.thrift.md5
@@ -0,0 +1 @@
+a8e77d5255804a8a745d20ca53b6aeda