Test Coverage for TradWeight using an Rset. #7

Closed
wants to merge 4 commits into
from

Projects

None yet

2 participants

@aarshkshah1992

I have added a test which tests the weighting of documents by TradWeight when an Rset is passed to it.

@ojwb ojwb and 1 other commented on an outdated diff Mar 4, 2013
xapian-core/ChangeLog
@@ -1,6 +1,11 @@
+Fri Mar 1 03:58:44 GMT +5:30 2013 Aarsh Shah <aarshkshah1992@gmail.com>
@ojwb
ojwb Mar 4, 2013 Contributor

We decided to make ChangeLog timestamps all GMT, as it seemed less confusing overall - HACKING gives a command to generate the preferred date format, and there's actually a script "svn-ci" in xapian-maintainer-tools which should help you write changelog entries.

@aarshkshah1992
aarshkshah1992 Mar 4, 2013

Yeah, saw the date --utc command in the HACKING document and copied the svn-ci script in my xapian-core directory ,will use it from now on .

@ojwb ojwb and 1 other commented on an outdated diff Mar 4, 2013
xapian-core/ChangeLog
@@ -1,6 +1,11 @@
+Fri Mar 1 03:58:44 GMT +5:30 2013 Aarsh Shah <aarshkshah1992@gmail.com>
+
+ * tests/api_anydb.cc: Added a tradweight4 test to test TadWeight with
+ an Rset for weightng documents.
@ojwb
ojwb Mar 4, 2013 Contributor

Please indent changelog entries with a tab character rather 8 spaces.

Also, take care with spelling - TadWeight -> TradWeight; Rset -> RSet; weightng -> weighting.

@aarshkshah1992
aarshkshah1992 Mar 4, 2013

Again, irresponsible on my part, will take care of spellings in all entries from now on.

@ojwb ojwb commented on the diff Mar 4, 2013
xapian-core/ChangeLog
Tue Feb 19 04:17:19 GMT 2013 Olly Betts <olly@survex.com>
- * common/Tokeniseise.pm: Add the ability to append lines to the
+ * common/Tokeniseise.pm: Add the ability to append lines to the
@ojwb
ojwb Mar 4, 2013 Contributor

And take care not to damage existing entries...

@aarshkshah1992
aarshkshah1992 Mar 4, 2013

Sorry, made the ChangeLog entry in a hurry ,it's irresponsible on my part , wont happen again .

@ojwb ojwb and 1 other commented on an outdated diff Mar 4, 2013
xapian-core/tests/api_anydb.cc
+DEFINE_TESTCASE(tradweight4, backend) {
+ Xapian::Database mydb(get_database("apitest_rset"));
+ Xapian::Enquire enquire(mydb);
+ Xapian::Query myquery = query(Xapian::Query::OP_OR, "cuddly", "people");
+
+ enquire.set_query(myquery);
+ enquire.set_weighting_scheme(Xapian::TradWeight());
+
+ Xapian::RSet myrset;
+ myrset.add_document(2);
+
+ Xapian::MSet mymset = enquire.get_mset(0, 10, &myrset);
+
+ /* Document 2 should have higher weight than Document 1 inspite of wdf of "people" being 1
+ because "people" indexes a document in the Rset where as "cuddly" (wdf=2) does not. */
+ mset_expect_order(mymset, 2, 1);
@ojwb
ojwb Mar 4, 2013 Contributor

The rset2 testcase also tests that without the RSet you get a different order, but you've removed that part. I think it would be useful to keep this - currently the tradweight4 testcase would pass if TradWeight ignored the RSet, but always gave the MSet order (2, 1).

@aarshkshah1992
aarshkshah1992 Mar 4, 2013

Yeah, I removed it because I thought that it was obvious that TradWeight would return (1,2) without an RSet .But now I understand what you say, this test will pass even if it doesn't use an RSet and a faulty TradWeight returns (2,1) .

@aarshkshah1992

I understand my mistake .As per the previous code,this test would have wrongly passed even if it did not use an RSet and a faulty TradWeight returned an MSet order of (2,1) .I just assumed that it was obvious that it would return (1,2) without an RSet but know now that it defeats the purpose of tests .

@aarshkshah1992
Owner

Sorry,had written the ChangeLog entry in a hurry. Now know of the date --utc "+%a %b %d %T GMT %Y" command and have copied the svn-ci script in my xapian-core directory.Will use it for all entries from now on .

@ojwb
Contributor
ojwb commented Mar 7, 2013

Patch merged. Thanks for you contribution!

@ojwb ojwb closed this Mar 7, 2013
@aarshkshah1992

Thank you so much :)

On Thu, Mar 7, 2013 at 6:17 AM, Olly Betts notifications@github.com wrote:

Patch merged. Thanks for you contribution!


Reply to this email directly or view it on GitHubhttps://github.com/xapian/xapian/pull/7#issuecomment-14536349
.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment