From 3dc8a03d7271c659139c157bb658795dd21298f9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 13:22:18 +0000 Subject: Remove history entries. When a chrome history entry is removed, remove that entry from our history too. Same when the entire history is removed. --- tests/unit_tests/completion_test.coffee | 37 ++++++++++++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) (limited to 'tests') diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 3de1d716..77464b96 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -41,6 +41,14 @@ context "HistoryCache", should "return length - 1 if it should be at the end of the list", -> assert.equal 0, HistoryCache.binarySearch(3, [3, 5, 8], @compare) + # FIXME (smblott) + # The following should pass, but fails. + # I think it's what the test above intends (but misses because of a typo). + # Bottom line: binarySearch can currently return an index beyond the end of the array. + # + # should "return end of list if greater than last element in list", -> + # assert.equal 2, HistoryCache.binarySearch(10, [3, 5, 8], @compare) + should "found return the position if it's between two elements", -> assert.equal 1, HistoryCache.binarySearch(4, [3, 5, 8], @compare) assert.equal 2, HistoryCache.binarySearch(7, [3, 5, 8], @compare) @@ -51,9 +59,11 @@ context "HistoryCache", @history2 = { url: "a.com", lastVisitTime: 10 } history = [@history1, @history2] @onVisitedListener = null + @onVisitRemovedListerner = null global.chrome.history = search: (options, callback) -> callback(history) onVisited: { addListener: (@onVisitedListener) => } + onVisitRemoved: { addListener: (@onVisitRemovedListerner) => } HistoryCache.reset() should "store visits sorted by url ascending", -> @@ -75,6 +85,30 @@ context "HistoryCache", HistoryCache.use (@results) => assert.arrayEqual [newSite, @history1], @results + should "remove pages from the history, when page is not in history", -> + HistoryCache.use (@results) => + assert.arrayEqual [@history2, @history1], @results + toRemove = { urls: [ "x.com" ], allHistory: false } + @onVisitRemovedListerner(toRemove) + HistoryCache.use (@results) => + assert.arrayEqual [@history2, @history1], @results + + should "remove pages from the history", -> + HistoryCache.use (@results) => + assert.arrayEqual [@history2, @history1], @results + toRemove = { urls: [ "a.com" ], allHistory: false } + @onVisitRemovedListerner(toRemove) + HistoryCache.use (@results) => + assert.arrayEqual [@history1], @results + + should "remove all pages from the history", -> + HistoryCache.use (@results) => + assert.arrayEqual [@history2, @history1], @results + toRemove = { allHistory: true } + @onVisitRemovedListerner(toRemove) + HistoryCache.use (@results) => + assert.arrayEqual [], @results + context "history completer", setup -> @history1 = { title: "history1", url: "history1.com", lastVisitTime: hours(1) } @@ -83,6 +117,7 @@ context "history completer", global.chrome.history = search: (options, callback) => callback([@history1, @history2]) onVisited: { addListener: -> } + onVisitRemoved: { addListener: -> } @completer = new HistoryCompleter() @@ -102,7 +137,7 @@ context "domain completer", @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2])) - global.chrome.history = { onVisited: { addListener: -> } } + global.chrome.history = { onVisited: { addListener: -> }, onVisitRemoved: -> } stub(Date, "now", returns(hours(24))) @completer = new DomainCompleter() -- cgit v1.2.3 From 03823f97a526ed5e5104b724d4f2a0b582505a6c Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 14:10:58 +0000 Subject: Extend removing entries to the domain completer. --- tests/unit_tests/completion_test.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 77464b96..5d7be78b 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -137,7 +137,7 @@ context "domain completer", @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2])) - global.chrome.history = { onVisited: { addListener: -> }, onVisitRemoved: -> } + global.chrome.history = { onVisited: { addListener: -> }, onVisitRemoved: { addListener: -> } } stub(Date, "now", returns(hours(24))) @completer = new DomainCompleter() @@ -147,7 +147,7 @@ context "domain completer", assert.arrayEqual ["history1.com"], results.map (result) -> result.url should "pick domains which are more recent", -> - # This domains are the same except for their last visited time. + # These domains are the same except for their last visited time. assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url @history2.lastVisitTime = hours(3) assert.equal "history2.com", filterCompleter(@completer, ["story"])[0].url -- cgit v1.2.3 From 2071873f5c9f496e4fde94d44cbf8b0e50bf4c21 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 15:59:27 +0000 Subject: Remove comments regarding non-existent bug. --- tests/unit_tests/completion_test.coffee | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) (limited to 'tests') diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 5d7be78b..f0fc9ded 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -41,13 +41,8 @@ context "HistoryCache", should "return length - 1 if it should be at the end of the list", -> assert.equal 0, HistoryCache.binarySearch(3, [3, 5, 8], @compare) - # FIXME (smblott) - # The following should pass, but fails. - # I think it's what the test above intends (but misses because of a typo). - # Bottom line: binarySearch can currently return an index beyond the end of the array. - # - # should "return end of list if greater than last element in list", -> - # assert.equal 2, HistoryCache.binarySearch(10, [3, 5, 8], @compare) + should "return one passed end of list (so: list.length) if greater than last element in list", -> + assert.equal 3, HistoryCache.binarySearch(10, [3, 5, 8], @compare) should "found return the position if it's between two elements", -> assert.equal 1, HistoryCache.binarySearch(4, [3, 5, 8], @compare) -- cgit v1.2.3 From 775122483b5c5dd7631e247bc3bb7c89aac0e9ab Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 17:33:29 +0000 Subject: Additional unit tests for domain completer. --- tests/unit_tests/completion_test.coffee | 35 +++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index f0fc9ded..6e6a2efb 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -54,11 +54,11 @@ context "HistoryCache", @history2 = { url: "a.com", lastVisitTime: 10 } history = [@history1, @history2] @onVisitedListener = null - @onVisitRemovedListerner = null + @onVisitRemovedListener = null global.chrome.history = search: (options, callback) -> callback(history) onVisited: { addListener: (@onVisitedListener) => } - onVisitRemoved: { addListener: (@onVisitRemovedListerner) => } + onVisitRemoved: { addListener: (@onVisitRemovedListener) => } HistoryCache.reset() should "store visits sorted by url ascending", -> @@ -84,7 +84,7 @@ context "HistoryCache", HistoryCache.use (@results) => assert.arrayEqual [@history2, @history1], @results toRemove = { urls: [ "x.com" ], allHistory: false } - @onVisitRemovedListerner(toRemove) + @onVisitRemovedListener(toRemove) HistoryCache.use (@results) => assert.arrayEqual [@history2, @history1], @results @@ -92,7 +92,7 @@ context "HistoryCache", HistoryCache.use (@results) => assert.arrayEqual [@history2, @history1], @results toRemove = { urls: [ "a.com" ], allHistory: false } - @onVisitRemovedListerner(toRemove) + @onVisitRemovedListener(toRemove) HistoryCache.use (@results) => assert.arrayEqual [@history1], @results @@ -100,7 +100,7 @@ context "HistoryCache", HistoryCache.use (@results) => assert.arrayEqual [@history2, @history1], @results toRemove = { allHistory: true } - @onVisitRemovedListerner(toRemove) + @onVisitRemovedListener(toRemove) HistoryCache.use (@results) => assert.arrayEqual [], @results @@ -132,7 +132,11 @@ context "domain completer", @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2])) - global.chrome.history = { onVisited: { addListener: -> }, onVisitRemoved: { addListener: -> } } + @onVisitedListener = null + @onVisitRemovedListener = null + global.chrome.history = + onVisited: { addListener: (@onVisitedListener) => } + onVisitRemoved: { addListener: (@onVisitRemovedListener) => } stub(Date, "now", returns(hours(24))) @completer = new DomainCompleter() @@ -150,6 +154,25 @@ context "domain completer", should "returns no results when there's more than one query term, because clearly it's not a domain", -> assert.arrayEqual [], filterCompleter(@completer, ["his", "tory"]) + should "remove 1 matching domain entry", -> + # Force installation of `@onVisitRemovedListener` + @history2.lastVisitTime = hours(3) and filterCompleter(@completer, ["story"]) + @onVisitRemovedListener { allHistory: false, urls: [ @history2.url ] } + assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url + + should "remove 2 (both) matching domain entries", -> + # Force installation of `@onVisitRemovedListener` + @history2.lastVisitTime = hours(3) and filterCompleter(@completer, ["story"]) + @onVisitRemovedListener { allHistory: false, urls: [ @history2.url ] } + @onVisitRemovedListener { allHistory: false, urls: [ @history1.url ] } + assert.isTrue filterCompleter(@completer, ["story"]).length == 0 + + should "remove *all* domain entries", -> + # Force installation of `@onVisitRemovedListener` + @history2.lastVisitTime = hours(3) and filterCompleter(@completer, ["story"]) + @onVisitRemovedListener { allHistory: true } + assert.isTrue filterCompleter(@completer, ["story"]).length == 0 + context "tab completer", setup -> @tabs = [ -- cgit v1.2.3 From cf757fc841c6b97a8341472bd15bdc95c527babc Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 17:45:24 +0000 Subject: More domain completer tests. --- tests/unit_tests/completion_test.coffee | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) (limited to 'tests') diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 6e6a2efb..e9e60ff9 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -130,8 +130,9 @@ context "domain completer", setup -> @history1 = { title: "history1", url: "http://history1.com", lastVisitTime: hours(1) } @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } + @history3 = { title: "history2", url: "http://history2.com/something", lastVisitTime: hours(0) } - stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2])) + stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2, @history3])) @onVisitedListener = null @onVisitRemovedListener = null global.chrome.history = @@ -142,7 +143,7 @@ context "domain completer", @completer = new DomainCompleter() should "return only a single matching domain", -> - results = filterCompleter(@completer, ["story"]) + results = filterCompleter(@completer, ["story1"]) assert.arrayEqual ["history1.com"], results.map (result) -> result.url should "pick domains which are more recent", -> @@ -156,20 +157,30 @@ context "domain completer", should "remove 1 matching domain entry", -> # Force installation of `@onVisitRemovedListener` - @history2.lastVisitTime = hours(3) and filterCompleter(@completer, ["story"]) + filterCompleter(@completer, ["story"]) @onVisitRemovedListener { allHistory: false, urls: [ @history2.url ] } assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url - should "remove 2 (both) matching domain entries", -> - # Force installation of `@onVisitRemovedListener` - @history2.lastVisitTime = hours(3) and filterCompleter(@completer, ["story"]) + should "remove all entries from one domain", -> + filterCompleter(@completer, ["story"]) # Force installation of `@onVisitRemovedListener`. + @onVisitRemovedListener { allHistory: false, urls: [ @history2.url ] } + @onVisitRemovedListener { allHistory: false, urls: [ @history3.url ] } + assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url + + should "remove 3 (all) matching domain entries", -> + filterCompleter(@completer, ["story"]) # Force installation of `@onVisitRemovedListener`. @onVisitRemovedListener { allHistory: false, urls: [ @history2.url ] } @onVisitRemovedListener { allHistory: false, urls: [ @history1.url ] } + @onVisitRemovedListener { allHistory: false, urls: [ @history3.url ] } + assert.isTrue filterCompleter(@completer, ["story"]).length == 0 + + should "remove 3 (all) matching domain entries, and do it all at once", -> + filterCompleter(@completer, ["story"]) # Force installation of `@onVisitRemovedListener`. + @onVisitRemovedListener { allHistory: false, urls: [ @history2.url, @history1.url, @history3.url ] } assert.isTrue filterCompleter(@completer, ["story"]).length == 0 should "remove *all* domain entries", -> - # Force installation of `@onVisitRemovedListener` - @history2.lastVisitTime = hours(3) and filterCompleter(@completer, ["story"]) + filterCompleter(@completer, ["story"]) # Force installation of `@onVisitRemovedListener`. @onVisitRemovedListener { allHistory: true } assert.isTrue filterCompleter(@completer, ["story"]).length == 0 -- cgit v1.2.3 From c44f0217fbfc4df1ac6bffea4a07abe3930e634e Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 11 Nov 2012 22:15:10 +0000 Subject: Changes in responce to philc's recommendations. See: https://github.com/philc/vimium/pull/715 --- tests/unit_tests/completion_test.coffee | 48 +++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 20 deletions(-) (limited to 'tests') diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index e9e60ff9..47fe16b6 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -80,7 +80,7 @@ context "HistoryCache", HistoryCache.use (@results) => assert.arrayEqual [newSite, @history1], @results - should "remove pages from the history, when page is not in history", -> + should "(not) remove page from the history, when page is not in history (it should be a no-op)", -> HistoryCache.use (@results) => assert.arrayEqual [@history2, @history1], @results toRemove = { urls: [ "x.com" ], allHistory: false } @@ -130,20 +130,17 @@ context "domain completer", setup -> @history1 = { title: "history1", url: "http://history1.com", lastVisitTime: hours(1) } @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } - @history3 = { title: "history2", url: "http://history2.com/something", lastVisitTime: hours(0) } - stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2, @history3])) - @onVisitedListener = null - @onVisitRemovedListener = null + stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2])) global.chrome.history = - onVisited: { addListener: (@onVisitedListener) => } - onVisitRemoved: { addListener: (@onVisitRemovedListener) => } + onVisited: { addListener: -> } + onVisitRemoved: { addListener: -> } stub(Date, "now", returns(hours(24))) @completer = new DomainCompleter() should "return only a single matching domain", -> - results = filterCompleter(@completer, ["story1"]) + results = filterCompleter(@completer, ["story"]) assert.arrayEqual ["history1.com"], results.map (result) -> result.url should "pick domains which are more recent", -> @@ -155,32 +152,43 @@ context "domain completer", should "returns no results when there's more than one query term, because clearly it's not a domain", -> assert.arrayEqual [], filterCompleter(@completer, ["his", "tory"]) - should "remove 1 matching domain entry", -> - # Force installation of `@onVisitRemovedListener` +context "domain completer (removing entries)", + setup -> + @history1 = { title: "history1", url: "http://history1.com", lastVisitTime: hours(1) } + @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } + @history3 = { title: "history2", url: "http://history2.com/something", lastVisitTime: hours(0) } + + stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2, @history3])) + @onVisitedListener = null + @onVisitRemovedListener = null + global.chrome.history = + onVisited: { addListener: (@onVisitedListener) => } + onVisitRemoved: { addListener: (@onVisitRemovedListener) => } + stub(Date, "now", returns(hours(24))) + + @completer = new DomainCompleter() filterCompleter(@completer, ["story"]) - @onVisitRemovedListener { allHistory: false, urls: [ @history2.url ] } + + should "remove 1 matching domain entry", -> + @onVisitRemovedListener { allHistory: false, urls: [@history2.url] } assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url should "remove all entries from one domain", -> - filterCompleter(@completer, ["story"]) # Force installation of `@onVisitRemovedListener`. - @onVisitRemovedListener { allHistory: false, urls: [ @history2.url ] } - @onVisitRemovedListener { allHistory: false, urls: [ @history3.url ] } + @onVisitRemovedListener { allHistory: false, urls: [@history2.url] } + @onVisitRemovedListener { allHistory: false, urls: [@history3.url] } assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url should "remove 3 (all) matching domain entries", -> - filterCompleter(@completer, ["story"]) # Force installation of `@onVisitRemovedListener`. - @onVisitRemovedListener { allHistory: false, urls: [ @history2.url ] } - @onVisitRemovedListener { allHistory: false, urls: [ @history1.url ] } - @onVisitRemovedListener { allHistory: false, urls: [ @history3.url ] } + @onVisitRemovedListener { allHistory: false, urls: [@history2.url] } + @onVisitRemovedListener { allHistory: false, urls: [@history1.url] } + @onVisitRemovedListener { allHistory: false, urls: [@history3.url] } assert.isTrue filterCompleter(@completer, ["story"]).length == 0 should "remove 3 (all) matching domain entries, and do it all at once", -> - filterCompleter(@completer, ["story"]) # Force installation of `@onVisitRemovedListener`. @onVisitRemovedListener { allHistory: false, urls: [ @history2.url, @history1.url, @history3.url ] } assert.isTrue filterCompleter(@completer, ["story"]).length == 0 should "remove *all* domain entries", -> - filterCompleter(@completer, ["story"]) # Force installation of `@onVisitRemovedListener`. @onVisitRemovedListener { allHistory: true } assert.isTrue filterCompleter(@completer, ["story"]).length == 0 -- cgit v1.2.3 From f0347388264b0fb437489f4f021ba8c0dbcca8e3 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Mon, 12 Nov 2012 06:43:11 +0000 Subject: Improve unit test structure and coverage. --- tests/unit_tests/completion_test.coffee | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 47fe16b6..08ce46c2 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -41,7 +41,7 @@ context "HistoryCache", should "return length - 1 if it should be at the end of the list", -> assert.equal 0, HistoryCache.binarySearch(3, [3, 5, 8], @compare) - should "return one passed end of list (so: list.length) if greater than last element in list", -> + should "return one passed end of array (so: array.length) if greater than last element in array", -> assert.equal 3, HistoryCache.binarySearch(10, [3, 5, 8], @compare) should "found return the position if it's between two elements", -> @@ -154,9 +154,9 @@ context "domain completer", context "domain completer (removing entries)", setup -> - @history1 = { title: "history1", url: "http://history1.com", lastVisitTime: hours(1) } + @history1 = { title: "history1", url: "http://history1.com", lastVisitTime: hours(2) } @history2 = { title: "history2", url: "http://history2.com", lastVisitTime: hours(1) } - @history3 = { title: "history2", url: "http://history2.com/something", lastVisitTime: hours(0) } + @history3 = { title: "history2something", url: "http://history2.com/something", lastVisitTime: hours(0) } stub(HistoryCache, "use", (onComplete) => onComplete([@history1, @history2, @history3])) @onVisitedListener = null @@ -167,30 +167,34 @@ context "domain completer (removing entries)", stub(Date, "now", returns(hours(24))) @completer = new DomainCompleter() + # Force installation of listeners. filterCompleter(@completer, ["story"]) - should "remove 1 matching domain entry", -> - @onVisitRemovedListener { allHistory: false, urls: [@history2.url] } - assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url + should "remove 1 entry for domain with reference count of 1", -> + @onVisitRemovedListener { allHistory: false, urls: [@history1.url] } + assert.equal "history2.com", filterCompleter(@completer, ["story"])[0].url + assert.equal 0, filterCompleter(@completer, ["story1"]).length - should "remove all entries from one domain", -> + should "remove 2 entries for domain with reference count of 2", -> @onVisitRemovedListener { allHistory: false, urls: [@history2.url] } + assert.equal "history2.com", filterCompleter(@completer, ["story2"])[0].url @onVisitRemovedListener { allHistory: false, urls: [@history3.url] } + assert.equal 0, filterCompleter(@completer, ["story2"]).length assert.equal "history1.com", filterCompleter(@completer, ["story"])[0].url should "remove 3 (all) matching domain entries", -> @onVisitRemovedListener { allHistory: false, urls: [@history2.url] } @onVisitRemovedListener { allHistory: false, urls: [@history1.url] } @onVisitRemovedListener { allHistory: false, urls: [@history3.url] } - assert.isTrue filterCompleter(@completer, ["story"]).length == 0 + assert.equal 0, filterCompleter(@completer, ["story"]).length should "remove 3 (all) matching domain entries, and do it all at once", -> @onVisitRemovedListener { allHistory: false, urls: [ @history2.url, @history1.url, @history3.url ] } - assert.isTrue filterCompleter(@completer, ["story"]).length == 0 + assert.equal 0, filterCompleter(@completer, ["story"]).length should "remove *all* domain entries", -> @onVisitRemovedListener { allHistory: true } - assert.isTrue filterCompleter(@completer, ["story"]).length == 0 + assert.equal 0, filterCompleter(@completer, ["story"]).length context "tab completer", setup -> -- cgit v1.2.3