This removes more addVisit calls in Places tests.

I made the framework in head_autocomplete.js asynchronous, so that the individual tests could continue to work without modifications. I added a title argument to markTyped because the "addVisits" helper doesn't provide a way to keep the existing title for the page.

This hasn't changed significantly since comment 1.

Comment on attachment 683514 [details] [diff] [review] Updated patch Review of attachment 683514 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/places/tests/autocomplete/head_autocomplete.js @@ +160,5 @@ > // Store the page info for each uri > let gPages = []; > > +// Initialization tasks to be run before the next test > +let gInitTasks = []; nit: gNextTestSetupTasks ? Init may be misinterpreted as global init tasks @@ +265,5 @@ > > + Task.spawn(function () { > + // Iterate over all tasks and execute them > + for (let [, [fn, args]] in Iterator(gInitTasks)) { > + yield fn.apply(this, args); ok at this point I assume Task just continues if yield is not a promise @@ +270,5 @@ > + }; > + > + // Clean up to allow tests to register more functions. > + gInitTasks = []; > + trailing spaces before and after @@ +304,5 @@ > + for each (let uri in aURIs) { > + yield promiseAddVisits({ > + uri: toURI(kURIs[uri]), > + transition: TRANSITION_TYPED, > + title: kTitles[aTitle] it's unfortunate AddVisits makes up a title if we don't provide one. IIRC, updatePlaces allows you to not pass a title, and if so keeps whatever is in the database, but AddVisits covers that... We could pass a special const for title KEEP_EXISTING_TITLE(-1) and AddVisit could just not pass a title at all if such value is found. This should allow you to avoid having to modify the markTyped signature. Also, don't use for each...in with arrays, use for...of

(In reply to Marco Bonardo [:mak] from comment #3) > it's unfortunate AddVisits makes up a title if we don't provide one. > IIRC, updatePlaces allows you to not pass a title, and if so keeps whatever > is in the database, but AddVisits covers that... We could pass a special > const for title KEEP_EXISTING_TITLE(-1) and AddVisit could just not pass a > title at all if such value is found. This should allow you to avoid having > to modify the markTyped signature. I don't remember whether the behavior of AddVisit and updatePlaces with regard to titles was consistent, I remember that I tried the solution you suggest, and in some tests I had to make changes anyway, so I went for specifying the title in all the tests across all folders (except, of course, the tests for the behavior of updatePlaces with empty titles, for which updatePlaces would be called directly in any case, since it is the function being tested). So, all the tests now provide titles. I don't think modifying the markTyped signature here is a big deal, maybe it's also better.