Search returns milestone resource when only one word matches

[Ryan J Ollos]

For a long time I've been noticing that the milestones would almost always appear in my search results, even for cases that I wouldn't expect them to appear. It looks like a milestone will be returned in the search results when there is a match from only one word of a multiword phrase.

The following test was done to prove the case:

1. Generated 3 words at ​http://listofrandomwords.com: Semiphilosophic opportunistic sulphur
2. Created 4 tickets: one ticket with the phrase as the summary and three tickets containing one word of the phrase as the summary.
3. Created 4 tickets: one ticket with the phrase as the description and three tickets containing one word of the phrase as the description.
4. Created 4 milestones: one milestone with the phrase in the description and three milestones containing one word of the phrase in the description.
5. Created 4 wiki pages: one wiki page with the phrase in the content and three wiki pages containing one word of the phrase in the content.

Here are the search results:

[Peter Suter]

I think it was changed in [11525] and could be fixed by changing any()​ to all(). (untested)

[Christian Boos]

Good catch! Right, all() seems appropriate to reproduce the behavior of the former SQL query in trunk/trac/search/api.py@11528#L54​.

[Ryan J Ollos]

Thanks for providing a fix. I've prepared the change along with a unit test in log:rjollos.git:t11545.

[Ryan J Ollos]

Committed to 1.0-stable in [12593], merged to trunk in [12595].

[Jun Omae]

After [12593], I got the following.

======================================================================
ERROR: test_get_search_results_milestone_not_in_filters (trac.ticket.tests.roadmap.MilestoneModuleTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jun66j5/src/trac/edgewall/svn/branches/1.0-stable/trac/ticket/tests/roadmap.py", line 155, in setUp
    m.insert()
  File "/home/jun66j5/src/trac/edgewall/svn/branches/1.0-stable/trac/ticket/model.py", line 1042, in insert
    to_utimestamp(self.completed), self.description))
  File "/home/jun66j5/src/trac/edgewall/svn/branches/1.0-stable/trac/db/util.py", line 121, in execute
    cursor.execute(query, params)
  File "/home/jun66j5/src/trac/edgewall/svn/branches/1.0-stable/trac/db/util.py", line 54, in execute
    r = self.cursor.execute(sql_escape_percent(sql), args)
  File "/home/jun66j5/src/trac/edgewall/svn/branches/1.0-stable/trac/db/sqlite_backend.py", line 78, in execute
    result = PyFormatCursor.execute(self, *args)
  File "/home/jun66j5/src/trac/edgewall/svn/branches/1.0-stable/trac/db/sqlite_backend.py", line 56, in execute
    args or [])
  File "/home/jun66j5/src/trac/edgewall/svn/branches/1.0-stable/trac/db/sqlite_backend.py", line 48, in _rollback_on_error
    return function(self, *args, **kwargs)
IntegrityError: column name is not unique

======================================================================
FAIL: test_get_search_results_matches_all_terms (trac.ticket.tests.roadmap.MilestoneModuleTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jun66j5/src/trac/edgewall/svn/branches/1.0-stable/trac/ticket/tests/roadmap.py", line 174, in test_get_search_results_matches_all_terms
    self.assertEqual(1, len(results))
AssertionError: 1 != 2

I've investigated.

• We must call reset_db() in tearDown() in the case.
• If /usr/share/dict/words might not be existent, random_word() generates using 10 words at most.

trac/ticket/tests/roadmap.py

@@ -145 +145 @@
         self.env = EnvironmentStub()
         self.req = Mock(href=self.env.href, perm=MockPerm())
         self.mmodule = MilestoneModule(self.env)
-        self.terms = [random_word() for i in range(0, 3)]
+        terms = set()
+        while len(terms) != 3:
+            terms.add(random_word())
+        self.terms = list(terms)
         self.milestones = []
         for term in self.terms + [' '.join(self.terms)]:
             m = Milestone(self.env)
@@ -155 +158 @@
             m.insert()
             self.milestones.append(m)
 
+    def tearDown(self):
+        self.env.reset_db()
+
     def test_get_search_filters(self):
         filters = self.mmodule.get_search_filters(self.req)
         filters = list(filters)

[Ryan J Ollos]

Okay, thanks. Maybe we should add random_unique_words(word_count, min_length=1) to trac.tests.contentgen.

[Jun Omae]

Also, I noticed that the /usr/share/dict/words file has words which contains other words, e.g. in v.s. winch. That terms are not unique.

It would be simple to be hard-coded rather than random_word().

-        self.terms = [random_word() for i in range(0, 3)]
+        self.terms = ['MilestoneAlpha', 'MilestoneBeta', 'MilestoneGamma']

[Ryan J Ollos]

Committed to 1.0-stable in [12603] and merged to trunk in [12604].

[Ryan J Ollos]

I was confused as to why it didn't seem to be necessary to call reset_db in tearDown, but it appears that is due to the reset_db call in the EnvironmentStub constructor: branches/1.0-stable/trac/test.py@12531:311-312#L292​. Is there a reason to also call reset_db in tearDown?

I found the destroying parameter to be a bit confusing. It appears to mean, don't initialize the database because the EnvironmentStub object is being constructed to call destroy_db. At least, that is the use-case in trac.tests.functional.testenv:FunctionalTestEnvironment. Initially I assumed it meant, construct a EnvironmentStub object and destroy existing data in the db. I'll add an entry in the docstring to clarify the behavior.

[Jun Omae]

Is there a reason to also call reset_db in tearDown?

The failures have gone away on r12603. But I still think we should call the reset_db to keep the database clean.

All test cases should reset database in tearDown() if records are created by setUp() or the test methods of the test case. Also, I don't think that the next test case always calls reset_db() from EnvironmentStub().

[Ryan J Ollos]

Refactoring and call to reset_db in tearDown committed in [12607], merged in [12608]. EnvironmentStub docstring improved in [12609], merged in [12610].