Add tests for browser.history.search that use specific date ranges

RESOLVED FIXED in Firefox 50

Status

defect
P3
normal
RESOLVED FIXED
3 years ago
Last year

People

(Reporter: bsilverberg, Assigned: bsilverberg)

Tracking

unspecified
mozilla50
Dependency tree / graph

Firefox Tracking Flags

(firefox50 fixed)

Details

(Whiteboard: [history]triaged)

Attachments

(1 attachment)

Once `insert` lands in History.jsm (which is being implemented as part of bug 1265836) we can (and should) enhance the tests for history.search to add history with specific dates (via History.insert) and then search for specific date ranges.

Currently the tests for history.search only include tests which return no results for dates that are too early or too late as we don't have an easy way to add history using anything by the current time.
Assignee

Updated

3 years ago
Assignee: nobody → bob.silverberg
Status: NEW → ASSIGNED
Depends on: 1265836
Priority: -- → P3
Whiteboard: [history]triaged
Assignee

Updated

3 years ago
Blocks: 1208334
No longer blocks: 1265834
Assignee

Comment 1

3 years ago
Because bug 1265835 is using the same set of tests as this bug references, and because getVisits will also need test data with specific dates which will be added via History.insert, I am combining this bug with that one. It will become Part 1 of that bug.
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1265835
Assignee

Comment 2

3 years ago
After spending a bunch of time trying to create tests for both search and getVisits I've realized that I'm just over complicating things by trying to do that, and ending up with a test that's far less readable, so I'm reopening this to address it separately from bug 1265835.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
Assignee

Updated

3 years ago
Status: REOPENED → ASSIGNED
Assignee

Updated

3 years ago
Blocks: 1278538
Assignee

Updated

3 years ago
Blocks: 1272686
Comment on attachment 8760396 [details]
Bug 1271675 - Add tests for browser.history.search that use specific date ranges,

https://reviewboard.mozilla.org/r/57968/#review55286

The bug title says "add tests" but the patch only adds one test :-)

::: browser/components/extensions/test/browser/browser_ext_history.js:219
(Diff revision 1)
> -  is(results.length, 1, "history.search returned 1 result");
> +  is(results.length, 1, "history.search with maxResults returned 1 result");
>    checkResult(results, DOUBLE_VISIT_URL, 2);
>  
> +  results = yield extension.awaitMessage("date-range-search");
> +  is(results.length, 2, "history.search with a date range returned 2 result");
> +  checkResult(results, DOUBLE_VISIT_URL, 2);

why is the count here 2?  there are two visits but only one is in the range that you're searching for.
Attachment #8760396 - Flags: review?(aswan) → review+
Assignee

Comment 5

3 years ago
https://reviewboard.mozilla.org/r/57968/#review55286

Yeah, after writing the one I decided that it was enough to cover the functionality.

> why is the count here 2?  there are two visits but only one is in the range that you're searching for.

`history.search` returns `HistoryItem`s [1], not `VisitItem`s [2]. This means that the criteria is used to return a HistoryItem for any URL that was visited during the date range. Each HistoryItem, however, will contain a count of the number of times that the URL was visited *for all time*. It is a bit confusing, but that's how it works. Whenever a HistoryItem is returned, it always contains a visitCount property which is the count of *all* visits to that URL.

[1] https://developer.chrome.com/extensions/history#type-HistoryItem
[2] https://developer.chrome.com/extensions/history#type-VisitItem
Assignee

Comment 7

3 years ago
Comment on attachment 8760396 [details]
Bug 1271675 - Add tests for browser.history.search that use specific date ranges,

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/57968/diff/1-2/
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 8

3 years ago
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/f8bdceb55c9d
Add tests for browser.history.search that use specific date ranges. r=aswan
Keywords: checkin-needed

Comment 9

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/f8bdceb55c9d
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50

Updated

Last year
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.