574577 - commenter_pronoun, cc_pronoun, and contact_pronoun should all be merged

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, enhancement)

Description

[Max Kanat-Alexander]

All of the functions that do the basic pronoun stuff in Search.pm should probably get merged into one function, and they should all support %group%. (Right now Commenter does not support %group%.)

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

Comment 2

[Max Kanat-Alexander]

Comment on attachment 580446 [details] [diff] [review]
patch, v1

Review of attachment 580446 [details] [diff] [review]:
-----------------------------------------------------------------

Does this pass xt? The reason the code was written the way it was was so that it worked with all possible operators, and also under NOT.

Comment 3

[Frédéric Buclin]

(In reply to Max Kanat-Alexander from comment #2)
> Does this pass xt?

No idea. On a fresh 4.3 installation with no patch applied, the xt code doesn't work. I wonder how it manages to pass in Tinderbox.

Comment 4

[Frédéric Buclin]

(In reply to Frédéric Buclin from comment #3)
> No idea. On a fresh 4.3 installation with no patch applied, the xt code
> doesn't work. I wonder how it manages to pass in Tinderbox.

I take that back. xt tests work fine on a fresh installation. I used an already existing DB in $db_name in localconfig which had a different DB schema. :)

And with my patch applied, all tests pass too. Here is the output:

# BZ_WRITE_TESTS=1 prove -lf xt/ :: --add-custom-fields 2> details
xt/search.t .. ok
All tests successful.

Test Summary Report
-------------------
xt/search.t (Wstat: 0 Tests: 72963 Failed: 0)
  TODO passed:   238-242
Files=1, Tests=72963, 33 wallclock secs ( 8.06 usr  0.18 sys + 25.75 cusr  0.58 csys = 34.57 CPU)
Result: PASS

Bonus point, I see that some TODO tests now pass, but I have no idea which ones they are.

Comment 5

[Max Kanat-Alexander]

Comment on attachment 580446 [details] [diff] [review]
patch, v1

Review of attachment 580446 [details] [diff] [review]:
-----------------------------------------------------------------

All right! Hooray for making TODO tests pass!

If you don't want the xt Tinderbox to break, what you'll have to do is update KNOWN_BROKEN in xt/lib/Bugzilla/Test/Search/Constants.pm. You can figure out which TODO tests are passing by using a regex over the "prove -v" output of the test.

I seem to recall that there was some reason I had this code separate, originally, and had trouble merging it, but if the test isn't catching the problem, then it's hard to prove that anything is wrong.

Comment 6

[Frédéric Buclin]

I will look at these TODO tests which pass in a moment. I'm focus on blockers right now.

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Search.pm
Committed revision 8090.

Comment 7

[Frédéric Buclin]

Attachment: Fix xt/

I fixed TODO tests which now pass.

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified xt/lib/Bugzilla/Test/Search/Constants.pm
Committed revision 8104.