Adding tests for bookmark search in omnibox

Adding tests for bookmark search in omnibox

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=68813

[deepakg, 2010-12-07 19:58:57]

Added two more test, testBookmarkResultInNewTab and
testBookmarkResultInNewWindow.

These need three other methods to to test bookmarks results on omnibox:
_CheckIfBookmarkMatches, _CheckBookmarkResultForVariousInputs, _AddBookmark.

[Nirnimesh, 2010-12-09 00:52:21]

http://codereview.chromium.org/5666001/diff/1/omnibox.py
File omnibox.py (right):

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode55
omnibox.py:55: def _AddBookmark(self, url, title):
Move this closer to where it's actually used

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode62
omnibox.py:62: self.assertEqual(1, len(nodes))
Remove the asserts

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode229
omnibox.py:229: urlMatches = self._GetOmniboxMatchesFor(url, windex)
use named arg wherever possible.
windex=windex

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode229
omnibox.py:229: urlMatches = self._GetOmniboxMatchesFor(url, windex)
camelCase var names not allowed.
rename: url_matches

Repeat for titleMatches, partialURL, partialTitle

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode232
omnibox.py:232: titleMatches = self._GetOmniboxMatchesFor(title, windex)
use named arg
windex=windex

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode235
omnibox.py:235: split_url = url.split('/')
use split* family of functions in urllib

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode238
omnibox.py:238: partialURL = self._GetOmniboxMatchesFor(search_term[0], windex)
use named arg
windex=windex

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode242
omnibox.py:242: search_term = split_title[len(split_title) -1]
need space after -

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode243
omnibox.py:243: partialTitle = self._GetOmniboxMatchesFor(search_term, windex)
use named arg
windex=windex

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode246
omnibox.py:246: def _CheckIfBookmarkMatches(self, matches):
Rename: _VerifyHasBookmarkResult

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode247
omnibox.py:247: """Verify that the Omnibox options have the bookmark."""
Verify that we have a bookmark result

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode256
omnibox.py:256: url = self.GetFileURLForPath(os.path.join(self.DataDir(),
'title2.html'))
use self.GetFileURLForDataPath('title2.html')

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode262
omnibox.py:262: self._CheckBookmarkResultForVariousInputs(url, title, 0)
use named arg. tab_index=0

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode264
omnibox.py:264: def testBookmarkResultInNewWindow(self):
Most of this test is similar to the last one. 
Merge this with the previous test.

[deepakg, 2010-12-09 01:59:50]

Done the changes.

http://codereview.chromium.org/5666001/diff/1/omnibox.py
File omnibox.py (right):

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode55
omnibox.py:55: def _AddBookmark(self, url, title):
On 2010/12/09 00:52:21, Nirnimesh wrote:
> Move this closer to where it's actually used

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode62
omnibox.py:62: self.assertEqual(1, len(nodes))
On 2010/12/09 00:52:21, Nirnimesh wrote:
> Remove the asserts

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode229
omnibox.py:229: urlMatches = self._GetOmniboxMatchesFor(url, windex)
On 2010/12/09 00:52:21, Nirnimesh wrote:
> use named arg wherever possible.
> windex=windex

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode229
omnibox.py:229: urlMatches = self._GetOmniboxMatchesFor(url, windex)
On 2010/12/09 00:52:21, Nirnimesh wrote:
> camelCase var names not allowed.
> rename: url_matches
> 
> Repeat for titleMatches, partialURL, partialTitle

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode232
omnibox.py:232: titleMatches = self._GetOmniboxMatchesFor(title, windex)
On 2010/12/09 00:52:21, Nirnimesh wrote:
> use named arg
> windex=windex

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode235
omnibox.py:235: split_url = url.split('/')
On 2010/12/09 00:52:21, Nirnimesh wrote:
> use split* family of functions in urllib

Done.

Have done this but urlparse.urlsplit would split the url into scheme, netloc and
path.

and the path is the complete path of the file eg: /Users/.../data/title.html all
it removes is the file://

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode238
omnibox.py:238: partialURL = self._GetOmniboxMatchesFor(search_term[0], windex)
On 2010/12/09 00:52:21, Nirnimesh wrote:
> use named arg
> windex=windex

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode242
omnibox.py:242: search_term = split_title[len(split_title) -1]
On 2010/12/09 00:52:21, Nirnimesh wrote:
> need space after -

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode243
omnibox.py:243: partialTitle = self._GetOmniboxMatchesFor(search_term, windex)
On 2010/12/09 00:52:21, Nirnimesh wrote:
> use named arg
> windex=windex

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode246
omnibox.py:246: def _CheckIfBookmarkMatches(self, matches):
On 2010/12/09 00:52:21, Nirnimesh wrote:
> Rename: _VerifyHasBookmarkResult

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode247
omnibox.py:247: """Verify that the Omnibox options have the bookmark."""
On 2010/12/09 00:52:21, Nirnimesh wrote:
> Verify that we have a bookmark result

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode256
omnibox.py:256: url = self.GetFileURLForPath(os.path.join(self.DataDir(),
'title2.html'))
On 2010/12/09 00:52:21, Nirnimesh wrote:
> use self.GetFileURLForDataPath('title2.html')

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode262
omnibox.py:262: self._CheckBookmarkResultForVariousInputs(url, title, 0)
On 2010/12/09 00:52:21, Nirnimesh wrote:
> use named arg. tab_index=0

Done.

http://codereview.chromium.org/5666001/diff/1/omnibox.py#newcode264
omnibox.py:264: def testBookmarkResultInNewWindow(self):
On 2010/12/09 00:52:21, Nirnimesh wrote:
> Most of this test is similar to the last one. 
> Merge this with the previous test.

Done.

[Nirnimesh, 2010-12-10 01:50:04]

LGTM