aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject git commit: Add a test to encourage taking care with thrift changes.
Date Fri, 17 Jan 2014 18:50:25 GMT
Updated Branches:
  refs/heads/wfarner/16986 [created] 59b3b58a2


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.

Testing Done:
Simulating a mis-matched md5:
    $ echo 'foobar' > src/test/resources/org/apache/thermos/thermos_internal.thrift.md5

    $ ./gradlew build
    make: Nothing to be done for `all'.
    Golden checksum did not match for src/main/thrift/org/apache/thermos/thermos_internal.thrift
    Found a8e77d5255804a8a745d20ca53b6aeda, expected foobar

    !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
    This means you changed a thrift file.
    Please think carefully before you proceed!

    If you are changing an API or a storage schema you may need to
    take additional actions to such as providing a client and/or
    server-side migration strategy.  You may also need to bump the
    released version ID, following the guidelines at http://semver.org

    This test is not here to help you make those changes, but to
    remind you to take appropriate follow-up actions relating to your
    schema change.

    Once you are confident that you have appropriately supported this
    change in relevant code, you can fix this test by running
    sh src/test/sh/verify_thrift_checksum.sh -r
    !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

    FAILURE: Build failed with an exception.

    * Where:
    Build file '/Users/wfarner/Code/aurora/build.gradle' line: 292

    * What went wrong:
    A problem occurred evaluating root project 'aurora'.
    > Process 'command 'sh'' finished with non-zero exit value 1

    * Try:
    Run with --stacktrace option to get the stack trace. Run with --info or --debug option
to get more log output.

    BUILD FAILED

    Total time: 5.783 secs

Fixing the break:

    $ sh src/test/sh/verify_thrift_checksum.sh -r
    Warning: Operating in reset mode.  Rewriting golden checksum files.

    $ ./gradlew build
    make: Nothing to be done for `all'.
    All checksums match.
    :about
    :bootstrapThrift UP-TO-DATE
    :generateSources UP-TO-DATE
    :compileGeneratedJava UP-TO-DATE
    :processGeneratedResources UP-TO-DATE
    :generatedClasses UP-TO-DATE
    :compileJava UP-TO-DATE
    :processResources UP-TO-DATE
    :classes UP-TO-DATE
    :jar
    :assemble
    :checkstyleMain UP-TO-DATE
    :compileTestJava UP-TO-DATE
    :processTestResources UP-TO-DATE
    :testClasses UP-TO-DATE
    :checkstyleTest UP-TO-DATE
    :test UP-TO-DATE
    :jacocoTestReport UP-TO-DATE
    :check UP-TO-DATE
    :build

    BUILD SUCCESSFUL

    Total time: 15.495 secs

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/59b3b58a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/59b3b58a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/59b3b58a

Branch: refs/heads/wfarner/16986
Commit: 59b3b58a29e62e2693a765e9abfdc8722d7fb0fc
Parents: 97fba43
Author: Bill Farner <wfarner@apache.org>
Authored: Fri Jan 17 10:06:08 2014 -0800
Committer: Bill Farner <wfarner@apache.org>
Committed: Fri Jan 17 10:49:13 2014 -0800

----------------------------------------------------------------------
 build.gradle                                    |  9 ++-
 .../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 +
 .../org/apache/aurora/verify_thrift_checksum.sh | 78 ++++++++++++++++++++
 8 files changed, 92 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/59b3b58a/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index 4a0372f..becc377 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/sh/org/apache/aurora/verify_thrift_checksum.sh']
+  }
+}

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/59b3b58a/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/59b3b58a/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/59b3b58a/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/59b3b58a/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/59b3b58a/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/59b3b58a/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

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/59b3b58a/src/test/sh/org/apache/aurora/verify_thrift_checksum.sh
----------------------------------------------------------------------
diff --git a/src/test/sh/org/apache/aurora/verify_thrift_checksum.sh b/src/test/sh/org/apache/aurora/verify_thrift_checksum.sh
new file mode 100644
index 0000000..13ba103
--- /dev/null
+++ b/src/test/sh/org/apache/aurora/verify_thrift_checksum.sh
@@ -0,0 +1,78 @@
+#!/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
+with open('$file', 'rb') as f:
+  print(hashlib.md5(f.read()).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


Mime
View raw message