Expanding context is broken for prepare-ChangeLog in the code review tool.

https://bugs.webkit.org/show_bug.cgi?id=74458

Reviewed by Adam Barth.

* code-review-test.html:
-Moved all the tests into test* functions.
-Automated calling all test* functions.
-Added testIsChangeLog.
* code-review.js:
Made the check for whether it's a ChangeLog file more robust.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@102719 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Websites/bugs.webkit.org/ChangeLog b/Websites/bugs.webkit.org/ChangeLog
index 70e87c6..bef87a0 100644 (file)
--- a/Websites/bugs.webkit.org/ChangeLog
+++ b/Websites/bugs.webkit.org/ChangeLog
@@ -1,5 +1,19 @@
 2011-12-13  Ojan Vafai  <ojan@chromium.org>
 
+        Expanding context is broken for prepare-ChangeLog in the code review tool.
+        https://bugs.webkit.org/show_bug.cgi?id=74458
+
+        Reviewed by Adam Barth.
+
+        * code-review-test.html:
+        -Moved all the tests into test* functions.
+        -Automated calling all test* functions.
+        -Added testIsChangeLog.
+        * code-review.js:
+        Made the check for whether it's a ChangeLog file more robust.
+
+2011-12-13  Ojan Vafai  <ojan@chromium.org>
+
         Fix bug in the code review tool when readding a discarded comment
         https://bugs.webkit.org/show_bug.cgi?id=74450
 

diff --git a/Websites/bugs.webkit.org/code-review-test.html b/Websites/bugs.webkit.org/code-review-test.html
index e46ce64..a290773 100644 (file)
--- a/Websites/bugs.webkit.org/code-review-test.html
+++ b/Websites/bugs.webkit.org/code-review-test.html
@@ -21,57 +21,6 @@ function inherits(childConstructor, parentConstructor) {
   childConstructor.prototype.constructor = childConstructor;
 }
 
-function MockLocalStorage() {
-    this.localStorageStore = {};
-    this.log = [];
-
-    this.getItem = function(key) {
-        this.log.push('getItem:' + key);
-        return this.localStorageStore[key];
-    };
-
-    this.setItem = function(key, value) {
-        // For testing sake, consider having more than 2 items to exceed the storage quota.
-        if (Object.keys(this.localStorageStore).length > 2) {
-            this.log.push('QuotaExceeded on setItem:' + key + ',' + value);
-            throw "QuotaExceeded";
-        }
-        this.log.push('setItem:' + key + ',' + value);
-        this.localStorageStore[key] = value;
-    };
-
-    this.removeItem = function(key) {
-        this.log.push('removeItem:' + key);
-        delete this.localStorageStore[key];
-    };
-
-    this.log_string = function() {
-        return this.log.join('\n');
-    };
-
-    this.key = function(i) {
-        return Object.keys(this.localStorageStore)[i];
-    };
-
-    this.__defineGetter__('length', function() {
-        return Object.keys(this.localStorageStore).length;
-    });
-}
-
-function MockDraftCommentSaver(attachment_id, opt_localStorage) {
-    DraftCommentSaver.call(this, attachment_id, opt_localStorage);
-}
-
-inherits(MockDraftCommentSaver, DraftCommentSaver)
-
-MockDraftCommentSaver.prototype._json = function() {
-    return "{MOCK JSON}";
-}
-
-MockDraftCommentSaver.prototype._should_remove_comments = function(message) {
-    return false;
-}
-
 function log(msg) {
     document.getElementById('output').innerHTML += msg + '\n\n';
 }
@@ -90,97 +39,151 @@ function ASSERT_EQUAL(actual, expected) {
         log('FAIL:\ngot:\n' + actual + '\nexpected:\n' + expected + '');
 }
 
-var links = tracLinks('foo/bar/baz.cpp', '');
-ASSERT_EQUAL($(links).attr('href'), 'http://trac.webkit.org/log/trunk/foo/bar/baz.h');
-
-var links = tracLinks('foo/bar.cpp/qux.mm', '');
-ASSERT_EQUAL($(links).attr('href'), 'http://trac.webkit.org/log/trunk/foo/bar.cpp/qux.h');
-
-// Basic setItem.
-var ls = new MockLocalStorage();
-new MockDraftCommentSaver('1234', ls).save();
-ASSERT_EQUAL(ls.log_string(), 'setItem:draft-comments-for-attachment-1234,{MOCK JSON}');
-
-// Exceed quota, but succeed after erasing old reviews.
-var ls = new MockLocalStorage();
-ls.localStorageStore = {
-    'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
-    'draft-comments-for-attachment-2': '{"born-on": 100, "comments":[]}',
-    'draft-comments-for-attachment-3': '{"born-on": 100, "comments":[]}',
-    'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
-};
-new MockDraftCommentSaver('1234', ls).save();
-ASSERT_EQUAL(ls.log_string(), 'QuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}\ngetItem:draft-comments-for-attachment-1\ngetItem:draft-comments-for-attachment-2\ngetItem:draft-comments-for-attachment-3\ngetItem:draft-comments-for-attachment-4\nremoveItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-2\nremoveItem:draft-comments-for-attachment-3\nsetItem:draft-comments-for-attachment-1234,{MOCK JSON}');
-
-// Exceed quota after erasing old reviews and fail after prompt.
-var ls = new MockLocalStorage();
-ls.localStorageStore = {
-    'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
-    'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
-    'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
-    'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
-};
-var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
-mockDraftSaver.save();
-// Second save to ensure that we stop trying to save when we fail the prompt.
-mockDraftSaver.save();
-ASSERT_EQUAL(ls.log_string(), 'QuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}\ngetItem:draft-comments-for-attachment-1\ngetItem:draft-comments-for-attachment-2\ngetItem:draft-comments-for-attachment-3\ngetItem:draft-comments-for-attachment-4\nremoveItem:draft-comments-for-attachment-1\nQuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}');
-
-// Exceed quota after erasing old reviews, but succeed after prompt.
-var ls = new MockLocalStorage();
-ls.localStorageStore = {
-    'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
-    'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
-    'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
-    'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
-};
-var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
-mockDraftSaver._should_remove_comments = function() { return true; };
-mockDraftSaver.save();
-ASSERT_EQUAL(ls.log_string(), 'QuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}\ngetItem:draft-comments-for-attachment-1\ngetItem:draft-comments-for-attachment-2\ngetItem:draft-comments-for-attachment-3\ngetItem:draft-comments-for-attachment-4\nremoveItem:draft-comments-for-attachment-1\nQuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}\nremoveItem:draft-comments-for-attachment-2\nremoveItem:draft-comments-for-attachment-3\nremoveItem:draft-comments-for-attachment-4\nsetItem:draft-comments-for-attachment-1234,{MOCK JSON}');
-
-// Always exceeds quota, even after erasing all review comments. There should be no setItem calls.
-var ls = new MockLocalStorage();
-ls.setItem = function() {
-    this.log.push('QuotaExceeded on setItem:' + key + ',' + value);
-    throw "QuotaExceeded"; 
+function testTracLinks() {
+  var links = tracLinks('foo/bar/baz.cpp', '');
+  ASSERT_EQUAL($(links).attr('href'), 'http://trac.webkit.org/log/trunk/foo/bar/baz.h');
+
+  var links = tracLinks('foo/bar.cpp/qux.mm', '');
+  ASSERT_EQUAL($(links).attr('href'), 'http://trac.webkit.org/log/trunk/foo/bar.cpp/qux.h');
 }
-ls.localStorageStore = {
-    'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
-    'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
-    'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
-    'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
-};
-var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
-mockDraftSaver._should_remove_comments = function() { return true; };
-mockDraftSaver.save();
-// Second save to ensure that we stop trying to save when we fail the prompt.
-mockDraftSaver.save();
-ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\ngetItem:draft-comments-for-attachment-2\ngetItem:draft-comments-for-attachment-3\ngetItem:draft-comments-for-attachment-4\nremoveItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-2\nremoveItem:draft-comments-for-attachment-3\nremoveItem:draft-comments-for-attachment-4');
-
-var ls = new MockLocalStorage();
-ls.localStorageStore = {
-    'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
-    'draft-comments-for-attachment-2': '{"born-on": 100, "comments":[{"start_line_id": 1, "end_line_id": 2, "contents": "DUMMY CONTENTS"}, {"start_line_id": 3, "end_line_id": 4, "contents": "DUMMY CONTENTS 2"}]}'
-};
-var comments = new MockDraftCommentSaver('2', ls).saved_comments().comments;
-ASSERT_EQUAL(comments.length, 2);
-ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-2');
-
-var ls = new MockLocalStorage();
-ls.localStorageStore = {
-    'draft-comments-for-attachment-1': 'corrupt comments'
-};
-var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
-ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
-
-var ls = new MockLocalStorage();
-ls.localStorageStore = {
-    'draft-comments-for-attachment-1': '["also corrupt comments"]'
-};
-var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
-ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
 
+function testDraftCommentSaver() {
+  function MockLocalStorage() {
+      this.localStorageStore = {};
+      this.log = [];
+
+      this.getItem = function(key) {
+          this.log.push('getItem:' + key);
+          return this.localStorageStore[key];
+      };
+
+      this.setItem = function(key, value) {
+          // For testing sake, consider having more than 2 items to exceed the storage quota.
+          if (Object.keys(this.localStorageStore).length > 2) {
+              this.log.push('QuotaExceeded on setItem:' + key + ',' + value);
+              throw "QuotaExceeded";
+          }
+          this.log.push('setItem:' + key + ',' + value);
+          this.localStorageStore[key] = value;
+      };
+
+      this.removeItem = function(key) {
+          this.log.push('removeItem:' + key);
+          delete this.localStorageStore[key];
+      };
+
+      this.log_string = function() {
+          return this.log.join('\n');
+      };
+
+      this.key = function(i) {
+          return Object.keys(this.localStorageStore)[i];
+      };
+
+      this.__defineGetter__('length', function() {
+          return Object.keys(this.localStorageStore).length;
+      });
+  }
+
+  function MockDraftCommentSaver(attachment_id, opt_localStorage) {
+      DraftCommentSaver.call(this, attachment_id, opt_localStorage);
+  }
+
+  inherits(MockDraftCommentSaver, DraftCommentSaver)
+
+  MockDraftCommentSaver.prototype._json = function() {
+      return "{MOCK JSON}";
+  }
+
+  MockDraftCommentSaver.prototype._should_remove_comments = function(message) {
+      return false;
+  }
+
+  // Basic setItem.
+  var ls = new MockLocalStorage();
+  new MockDraftCommentSaver('1234', ls).save();
+  ASSERT_EQUAL(ls.log_string(), 'setItem:draft-comments-for-attachment-1234,{MOCK JSON}');
+
+  // Exceed quota, but succeed after erasing old reviews.
+  var ls = new MockLocalStorage();
+  ls.localStorageStore = {
+      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
+      'draft-comments-for-attachment-2': '{"born-on": 100, "comments":[]}',
+      'draft-comments-for-attachment-3': '{"born-on": 100, "comments":[]}',
+      'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
+  };
+  new MockDraftCommentSaver('1234', ls).save();
+  ASSERT_EQUAL(ls.log_string(), 'QuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}\ngetItem:draft-comments-for-attachment-1\ngetItem:draft-comments-for-attachment-2\ngetItem:draft-comments-for-attachment-3\ngetItem:draft-comments-for-attachment-4\nremoveItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-2\nremoveItem:draft-comments-for-attachment-3\nsetItem:draft-comments-for-attachment-1234,{MOCK JSON}');
+
+  // Exceed quota after erasing old reviews and fail after prompt.
+  var ls = new MockLocalStorage();
+  ls.localStorageStore = {
+      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
+      'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
+      'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
+      'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
+  };
+  var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
+  mockDraftSaver.save();
+  // Second save to ensure that we stop trying to save when we fail the prompt.
+  mockDraftSaver.save();
+  ASSERT_EQUAL(ls.log_string(), 'QuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}\ngetItem:draft-comments-for-attachment-1\ngetItem:draft-comments-for-attachment-2\ngetItem:draft-comments-for-attachment-3\ngetItem:draft-comments-for-attachment-4\nremoveItem:draft-comments-for-attachment-1\nQuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}');
+
+  // Exceed quota after erasing old reviews, but succeed after prompt.
+  var ls = new MockLocalStorage();
+  ls.localStorageStore = {
+      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
+      'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
+      'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
+      'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
+  };
+  var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
+  mockDraftSaver._should_remove_comments = function() { return true; };
+  mockDraftSaver.save();
+  ASSERT_EQUAL(ls.log_string(), 'QuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}\ngetItem:draft-comments-for-attachment-1\ngetItem:draft-comments-for-attachment-2\ngetItem:draft-comments-for-attachment-3\ngetItem:draft-comments-for-attachment-4\nremoveItem:draft-comments-for-attachment-1\nQuotaExceeded on setItem:draft-comments-for-attachment-1234,{MOCK JSON}\nremoveItem:draft-comments-for-attachment-2\nremoveItem:draft-comments-for-attachment-3\nremoveItem:draft-comments-for-attachment-4\nsetItem:draft-comments-for-attachment-1234,{MOCK JSON}');
+
+  // Always exceeds quota, even after erasing all review comments. There should be no setItem calls.
+  var ls = new MockLocalStorage();
+  ls.setItem = function() {
+      this.log.push('QuotaExceeded on setItem:' + key + ',' + value);
+      throw "QuotaExceeded"; 
+  }
+  ls.localStorageStore = {
+      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
+      'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
+      'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
+      'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
+  };
+  var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
+  mockDraftSaver._should_remove_comments = function() { return true; };
+  mockDraftSaver.save();
+  // Second save to ensure that we stop trying to save when we fail the prompt.
+  mockDraftSaver.save();
+  ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\ngetItem:draft-comments-for-attachment-2\ngetItem:draft-comments-for-attachment-3\ngetItem:draft-comments-for-attachment-4\nremoveItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-2\nremoveItem:draft-comments-for-attachment-3\nremoveItem:draft-comments-for-attachment-4');
+
+  var ls = new MockLocalStorage();
+  ls.localStorageStore = {
+      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
+      'draft-comments-for-attachment-2': '{"born-on": 100, "comments":[{"start_line_id": 1, "end_line_id": 2, "contents": "DUMMY CONTENTS"}, {"start_line_id": 3, "end_line_id": 4, "contents": "DUMMY CONTENTS 2"}]}'
+  };
+  var comments = new MockDraftCommentSaver('2', ls).saved_comments().comments;
+  ASSERT_EQUAL(comments.length, 2);
+  ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-2');
+
+  var ls = new MockLocalStorage();
+  ls.localStorageStore = {
+      'draft-comments-for-attachment-1': 'corrupt comments'
+  };
+  var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
+  ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
+
+  var ls = new MockLocalStorage();
+  ls.localStorageStore = {
+      'draft-comments-for-attachment-1': '["also corrupt comments"]'
+  };
+  var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
+  ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
+}
 
 function stripBornOn(json) {
   return json.replace(/\"born\-on\"\:\d+,/, "");
@@ -257,5 +260,17 @@ function testReaddDiscardedCommentWithPreviousComment() {
 
   document.getElementById('diff-content').innerHTML = '';
 }
-testReaddDiscardedCommentWithPreviousComment();
+
+function testIsChangeLog() {
+  ASSERT("Top-level ChangeLog file is a ChangeLog file", isChangeLog("ChangeLog"));
+  ASSERT("Second-level ChangeLog file is a ChangeLog file", isChangeLog("Tools/ChangeLog"));
+  ASSERT("prepare-ChangeLog is not a ChangeLog file", !isChangeLog("Tools/Scripts/prepare-ChangeLog"));
+  ASSERT("ChangeLog-script is not a ChangeLog file", !isChangeLog("Tools/Scripts/ChangeLog-script"));
+}
+
+for (var property in window) {
+  if (property.indexOf('test') == 0) {
+    window[property]();
+  }
+}
 </script>

diff --git a/Websites/bugs.webkit.org/code-review.js b/Websites/bugs.webkit.org/code-review.js
index 78223bd..14322f3 100644 (file)
--- a/Websites/bugs.webkit.org/code-review.js
+++ b/Websites/bugs.webkit.org/code-review.js
@@ -642,8 +642,12 @@ var CODE_REVIEW_UNITTEST;
     return trac_links;
   }
 
+  function isChangeLog(file_name) {
+    return file_name.match(/\/ChangeLog$/) || file_name == 'ChangeLog';
+  }
+
   function addExpandLinks(file_name) {
-    if (file_name.indexOf('ChangeLog') != -1)
+    if (isChangeLog(file_name))
       return;
 
     var file_diff = files[file_name];
@@ -1994,6 +1998,7 @@ var CODE_REVIEW_UNITTEST;
     window.eraseDraftComments = eraseDraftComments;
     window.unfreezeComment = unfreezeComment;
     window.g_draftCommentSaver = g_draftCommentSaver;
+    window.isChangeLog = isChangeLog;
   } else {
     $(document).ready(handleDocumentReady)
   }