Changeset 102719 in webkit


Ignore:
Timestamp:
Dec 13, 2011 4:07:22 PM (12 years ago)
Author:
ojan@chromium.org
Message:

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.

Location:
trunk/Websites/bugs.webkit.org
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Websites/bugs.webkit.org/ChangeLog

    r102711 r102719  
     12011-12-13  Ojan Vafai  <ojan@chromium.org>
     2
     3        Expanding context is broken for prepare-ChangeLog in the code review tool.
     4        https://bugs.webkit.org/show_bug.cgi?id=74458
     5
     6        Reviewed by Adam Barth.
     7
     8        * code-review-test.html:
     9        -Moved all the tests into test* functions.
     10        -Automated calling all test* functions.
     11        -Added testIsChangeLog.
     12        * code-review.js:
     13        Made the check for whether it's a ChangeLog file more robust.
     14
    1152011-12-13  Ojan Vafai  <ojan@chromium.org>
    216

  • trunk/Websites/bugs.webkit.org/code-review-test.html

    r102711 r102719  
    2222}
    2323
    24 function MockLocalStorage() {
    25     this.localStorageStore = {};
    26     this.log = [];
    27 
    28     this.getItem = function(key) {
    29         this.log.push('getItem:' + key);
    30         return this.localStorageStore[key];
    31     };
    32 
    33     this.setItem = function(key, value) {
    34         // For testing sake, consider having more than 2 items to exceed the storage quota.
    35         if (Object.keys(this.localStorageStore).length > 2) {
    36             this.log.push('QuotaExceeded on setItem:' + key + ',' + value);
    37             throw "QuotaExceeded";
    38         }
    39         this.log.push('setItem:' + key + ',' + value);
    40         this.localStorageStore[key] = value;
    41     };
    42 
    43     this.removeItem = function(key) {
    44         this.log.push('removeItem:' + key);
    45         delete this.localStorageStore[key];
    46     };
    47 
    48     this.log_string = function() {
    49         return this.log.join('\n');
    50     };
    51 
    52     this.key = function(i) {
    53         return Object.keys(this.localStorageStore)[i];
    54     };
    55 
    56     this.__defineGetter__('length', function() {
    57         return Object.keys(this.localStorageStore).length;
    58     });
    59 }
    60 
    61 function MockDraftCommentSaver(attachment_id, opt_localStorage) {
    62     DraftCommentSaver.call(this, attachment_id, opt_localStorage);
    63 }
    64 
    65 inherits(MockDraftCommentSaver, DraftCommentSaver)
    66 
    67 MockDraftCommentSaver.prototype._json = function() {
    68     return "{MOCK JSON}";
    69 }
    70 
    71 MockDraftCommentSaver.prototype._should_remove_comments = function(message) {
    72     return false;
    73 }
    74 
    7524function log(msg) {
    7625    document.getElementById('output').innerHTML += msg + '\n\n';
    … …  
    9140}
    9241
    93 var links = tracLinks('foo/bar/baz.cpp', '');
    94 ASSERT_EQUAL($(links).attr('href'), 'http://trac.webkit.org/log/trunk/foo/bar/baz.h');
    95 
    96 var links = tracLinks('foo/bar.cpp/qux.mm', '');
    97 ASSERT_EQUAL($(links).attr('href'), 'http://trac.webkit.org/log/trunk/foo/bar.cpp/qux.h');
    98 
    99 // Basic setItem.
    100 var ls = new MockLocalStorage();
    101 new MockDraftCommentSaver('1234', ls).save();
    102 ASSERT_EQUAL(ls.log_string(), 'setItem:draft-comments-for-attachment-1234,{MOCK JSON}');
    103 
    104 // Exceed quota, but succeed after erasing old reviews.
    105 var ls = new MockLocalStorage();
    106 ls.localStorageStore = {
    107     'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
    108     'draft-comments-for-attachment-2': '{"born-on": 100, "comments":[]}',
    109     'draft-comments-for-attachment-3': '{"born-on": 100, "comments":[]}',
    110     'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
    111 };
    112 new MockDraftCommentSaver('1234', ls).save();
    113 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}');
    114 
    115 // Exceed quota after erasing old reviews and fail after prompt.
    116 var ls = new MockLocalStorage();
    117 ls.localStorageStore = {
    118     'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
    119     'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
    120     'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
    121     'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
    122 };
    123 var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
    124 mockDraftSaver.save();
    125 // Second save to ensure that we stop trying to save when we fail the prompt.
    126 mockDraftSaver.save();
    127 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}');
    128 
    129 // Exceed quota after erasing old reviews, but succeed after prompt.
    130 var ls = new MockLocalStorage();
    131 ls.localStorageStore = {
    132     'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
    133     'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
    134     'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
    135     'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
    136 };
    137 var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
    138 mockDraftSaver._should_remove_comments = function() { return true; };
    139 mockDraftSaver.save();
    140 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}');
    141 
    142 // Always exceeds quota, even after erasing all review comments. There should be no setItem calls.
    143 var ls = new MockLocalStorage();
    144 ls.setItem = function() {
    145     this.log.push('QuotaExceeded on setItem:' + key + ',' + value);
    146     throw "QuotaExceeded";
    147 }
    148 ls.localStorageStore = {
    149     'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
    150     'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
    151     'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
    152     'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
    153 };
    154 var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
    155 mockDraftSaver._should_remove_comments = function() { return true; };
    156 mockDraftSaver.save();
    157 // Second save to ensure that we stop trying to save when we fail the prompt.
    158 mockDraftSaver.save();
    159 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');
    160 
    161 var ls = new MockLocalStorage();
    162 ls.localStorageStore = {
    163     'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
    164     '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"}]}'
    165 };
    166 var comments = new MockDraftCommentSaver('2', ls).saved_comments().comments;
    167 ASSERT_EQUAL(comments.length, 2);
    168 ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-2');
    169 
    170 var ls = new MockLocalStorage();
    171 ls.localStorageStore = {
    172     'draft-comments-for-attachment-1': 'corrupt comments'
    173 };
    174 var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
    175 ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
    176 
    177 var ls = new MockLocalStorage();
    178 ls.localStorageStore = {
    179     'draft-comments-for-attachment-1': '["also corrupt comments"]'
    180 };
    181 var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
    182 ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
    183 
     42function testTracLinks() {
     43  var links = tracLinks('foo/bar/baz.cpp', '');
     44  ASSERT_EQUAL($(links).attr('href'), 'http://trac.webkit.org/log/trunk/foo/bar/baz.h');
     45
     46  var links = tracLinks('foo/bar.cpp/qux.mm', '');
     47  ASSERT_EQUAL($(links).attr('href'), 'http://trac.webkit.org/log/trunk/foo/bar.cpp/qux.h');
     48}
     49
     50function testDraftCommentSaver() {
     51  function MockLocalStorage() {
     52      this.localStorageStore = {};
     53      this.log = [];
     54
     55      this.getItem = function(key) {
     56          this.log.push('getItem:' + key);
     57          return this.localStorageStore[key];
     58      };
     59
     60      this.setItem = function(key, value) {
     61          // For testing sake, consider having more than 2 items to exceed the storage quota.
     62          if (Object.keys(this.localStorageStore).length > 2) {
     63              this.log.push('QuotaExceeded on setItem:' + key + ',' + value);
     64              throw "QuotaExceeded";
     65          }
     66          this.log.push('setItem:' + key + ',' + value);
     67          this.localStorageStore[key] = value;
     68      };
     69
     70      this.removeItem = function(key) {
     71          this.log.push('removeItem:' + key);
     72          delete this.localStorageStore[key];
     73      };
     74
     75      this.log_string = function() {
     76          return this.log.join('\n');
     77      };
     78
     79      this.key = function(i) {
     80          return Object.keys(this.localStorageStore)[i];
     81      };
     82
     83      this.__defineGetter__('length', function() {
     84          return Object.keys(this.localStorageStore).length;
     85      });
     86  }
     87
     88  function MockDraftCommentSaver(attachment_id, opt_localStorage) {
     89      DraftCommentSaver.call(this, attachment_id, opt_localStorage);
     90  }
     91
     92  inherits(MockDraftCommentSaver, DraftCommentSaver)
     93
     94  MockDraftCommentSaver.prototype._json = function() {
     95      return "{MOCK JSON}";
     96  }
     97
     98  MockDraftCommentSaver.prototype._should_remove_comments = function(message) {
     99      return false;
     100  }
     101
     102  // Basic setItem.
     103  var ls = new MockLocalStorage();
     104  new MockDraftCommentSaver('1234', ls).save();
     105  ASSERT_EQUAL(ls.log_string(), 'setItem:draft-comments-for-attachment-1234,{MOCK JSON}');
     106
     107  // Exceed quota, but succeed after erasing old reviews.
     108  var ls = new MockLocalStorage();
     109  ls.localStorageStore = {
     110      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
     111      'draft-comments-for-attachment-2': '{"born-on": 100, "comments":[]}',
     112      'draft-comments-for-attachment-3': '{"born-on": 100, "comments":[]}',
     113      'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
     114  };
     115  new MockDraftCommentSaver('1234', ls).save();
     116  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}');
     117
     118  // Exceed quota after erasing old reviews and fail after prompt.
     119  var ls = new MockLocalStorage();
     120  ls.localStorageStore = {
     121      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
     122      'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
     123      'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
     124      'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
     125  };
     126  var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
     127  mockDraftSaver.save();
     128  // Second save to ensure that we stop trying to save when we fail the prompt.
     129  mockDraftSaver.save();
     130  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}');
     131
     132  // Exceed quota after erasing old reviews, but succeed after prompt.
     133  var ls = new MockLocalStorage();
     134  ls.localStorageStore = {
     135      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
     136      'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
     137      'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
     138      'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
     139  };
     140  var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
     141  mockDraftSaver._should_remove_comments = function() { return true; };
     142  mockDraftSaver.save();
     143  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}');
     144
     145  // Always exceeds quota, even after erasing all review comments. There should be no setItem calls.
     146  var ls = new MockLocalStorage();
     147  ls.setItem = function() {
     148      this.log.push('QuotaExceeded on setItem:' + key + ',' + value);
     149      throw "QuotaExceeded";
     150  }
     151  ls.localStorageStore = {
     152      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
     153      'draft-comments-for-attachment-2': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
     154      'draft-comments-for-attachment-3': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}',
     155      'draft-comments-for-attachment-4': '{"born-on": ' + (Date.now() - 100) + ', "comments":[]}'
     156  };
     157  var mockDraftSaver = new MockDraftCommentSaver('1234', ls);
     158  mockDraftSaver._should_remove_comments = function() { return true; };
     159  mockDraftSaver.save();
     160  // Second save to ensure that we stop trying to save when we fail the prompt.
     161  mockDraftSaver.save();
     162  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');
     163
     164  var ls = new MockLocalStorage();
     165  ls.localStorageStore = {
     166      'draft-comments-for-attachment-1': '{"born-on": 100, "comments":[]}',
     167      '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"}]}'
     168  };
     169  var comments = new MockDraftCommentSaver('2', ls).saved_comments().comments;
     170  ASSERT_EQUAL(comments.length, 2);
     171  ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-2');
     172
     173  var ls = new MockLocalStorage();
     174  ls.localStorageStore = {
     175      'draft-comments-for-attachment-1': 'corrupt comments'
     176  };
     177  var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
     178  ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
     179
     180  var ls = new MockLocalStorage();
     181  ls.localStorageStore = {
     182      'draft-comments-for-attachment-1': '["also corrupt comments"]'
     183  };
     184  var comments = new MockDraftCommentSaver('1', ls).saved_comments().comments;
     185  ASSERT_EQUAL(ls.log_string(), 'getItem:draft-comments-for-attachment-1\nremoveItem:draft-comments-for-attachment-1');
     186}
    184187
    185188function stripBornOn(json) {
    … …  
    258261  document.getElementById('diff-content').innerHTML = '';
    259262}
    260 testReaddDiscardedCommentWithPreviousComment();
     263
     264function testIsChangeLog() {
     265  ASSERT("Top-level ChangeLog file is a ChangeLog file", isChangeLog("ChangeLog"));
     266  ASSERT("Second-level ChangeLog file is a ChangeLog file", isChangeLog("Tools/ChangeLog"));
     267  ASSERT("prepare-ChangeLog is not a ChangeLog file", !isChangeLog("Tools/Scripts/prepare-ChangeLog"));
     268  ASSERT("ChangeLog-script is not a ChangeLog file", !isChangeLog("Tools/Scripts/ChangeLog-script"));
     269}
     270
     271for (var property in window) {
     272  if (property.indexOf('test') == 0) {
     273    window[property]();
     274  }
     275}
    261276</script>

  • trunk/Websites/bugs.webkit.org/code-review.js

    r102711 r102719  
    643643  }
    644644
     645  function isChangeLog(file_name) {
     646    return file_name.match(/\/ChangeLog$/) || file_name == 'ChangeLog';
     647  }
     648
    645649  function addExpandLinks(file_name) {
    646     if (file_name.indexOf('ChangeLog') != -1)
     650    if (isChangeLog(file_name))
    647651      return;
    648652
    … …  
    19951999    window.unfreezeComment = unfreezeComment;
    19962000    window.g_draftCommentSaver = g_draftCommentSaver;
     2001    window.isChangeLog = isChangeLog;
    19972002  } else {
    19982003    $(document).ready(handleDocumentReady)
Note: See TracChangeset for help on using the changeset viewer.