Deduplication of server search results unittests #780
Conversation
Codecov Report
@@ Coverage Diff @@
## master #780 +/- ##
=======================================
Coverage 63.66% 63.66%
=======================================
Files 59 59
Lines 4073 4073
Branches 2218 2218
=======================================
Hits 2593 2593
Misses 1478 1478
Partials 2 2 Continue to review full report at Codecov.
|
| std::string s; | ||
| for ( const auto& r : results ) { | ||
| s += "\n <item>\n"; | ||
| s += maskSnippetsInXmlSearchResults(r.getXml()); |
There was a problem hiding this comment.
I wonder if we could generate xml (and html) without snippets instead of generated a full xml and mask the snippet.
There was a problem hiding this comment.
That would mean having a special mode for the /search endpoint (e.g. /search?snippets=no), however I don't think that adding it for testing alone makes sense. Besides, we ignore snippets only at this point. Later snippets are validated too.
There was a problem hiding this comment.
I was speaking about the generation of the expected result. We would still have to mask the snippet in the actual results of the tested server.
There was a problem hiding this comment.
That makes sense. But not to an extent to pursue it if this is going to be the only thing to fix about this PR.
There was a problem hiding this comment.
Frankly I would rather change TestData::expectedXmlResultsString() to return the real expected response body and perform the masking out of snippets at the check site. That will eliminate some confusion implanted in the current code.
There was a problem hiding this comment.
But not to an extent to pursue it if this is going to be the only thing to fix about this PR.
Agree with that.
Frankly I would rather change TestData::expectedXmlResultsString() to return the real expected response body and perform the masking out of snippets at the check site.
This is the other way around. Feel free to do it (but not in this PR, I approve and merge it)
Unit-tests of search results in XML format should work the same way with a server that would inject a taskbar into HTML responses. This small change actually validates that taskbar injection is disabled for XML responses.
Now ServerTest provides an optional taskbarless kiwix::Server.
Now that ServerTest.searchResults is in a separate cpp file, there are no reasons for hiding its test data definition inside the unit test function. The diff is much-much simpler if whitespace changes are ignored.
Included the word "Html" in the names of those functions and variables which will get Xml siblings soon.
mgautierfr
force-pushed
the
deduping_searchResults_unittests
branch
from
4f11dbe
to
baa97ca
Compare
A follow-up of #731 (comment)