Differential D11907 Diff 31512 autotests/unit/lib/advancedqueryparsertest.cpp

Changeset View

Standalone View

View Options

autotests/unit/lib/advancedqueryparsertest.cpp

Show All 16 Lines
17 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA 17​ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
18 * 18​ *
19 */ 19​ */
20 20​
21#include "advancedqueryparser.h" 21​#include "advancedqueryparser.h"
22 22​
23#include <QTest> 23​#include <QTest>
24 24​
25​Q_DECLARE_METATYPE(Baloo::Term)
25using namespace Baloo; 26​using namespace Baloo;
26 27​
27class AdvancedQueryParserTest : public QObject 28​class AdvancedQueryParserTest : public QObject
28{ 29​{
29 Q_OBJECT 30​ Q_OBJECT
30private Q_SLOTS: 31​private Q_SLOTS:
31 32​
32 void testSimpleProperty(); 33​ void testSimpleProperty();
33 void testSimpleString(); 34​ void testSimpleString();
34 void testStringAndProperty(); 35​ void testStringAndProperty();
35 void testLogicalOps(); 36​ void testLogicalOps();
36 void testNesting(); 37​ void testNesting();
37 void testDateTime(); 38​ void testDateTime();
38 void testOperators(); 39​ void testOperators();
39 void testBinaryOperatorMissingFirstArg(); 40​ void testBinaryOperatorMissingFirstArg();
41​ void testNestedParentheses();
42​ void testNestedParentheses_data();
43​ void testOptimizedLogic();
44​ void testOptimizedLogic_data();
40}; 45​};
41 46​
42void AdvancedQueryParserTest::testSimpleProperty() 47​void AdvancedQueryParserTest::testSimpleProperty()
43{ 48​{
44 AdvancedQueryParser parser; 49​ AdvancedQueryParser parser;
45 Term term = parser.parse(QStringLiteral("artist:Coldplay")); 50​ Term term = parser.parse(QStringLiteral("artist:Coldplay"));
46 Term expectedTerm(QStringLiteral("artist"), "Coldplay"); 51​ Term expectedTerm(QStringLiteral("artist"), "Coldplay");
47 52​
▲ Show 20 Lines • Show All 146 Lines • ▼ Show 20 Line(s)
194void AdvancedQueryParserTest::testBinaryOperatorMissingFirstArg() 199​void AdvancedQueryParserTest::testBinaryOperatorMissingFirstArg()
195{ 200​{
196 AdvancedQueryParser parser; 201​ AdvancedQueryParser parser;
197 Term term = parser.parse(QStringLiteral("=:2")); 202​ Term term = parser.parse(QStringLiteral("=:2"));
198 Term expectedTerm; 203​ Term expectedTerm;
199 QCOMPARE(term, expectedTerm); 204​ QCOMPARE(term, expectedTerm);
200} 205​}
201 206​
207​void AdvancedQueryParserTest::testNestedParentheses()
208​{
209​ QFETCH(QString, searchInput);
210​ QFETCH(QString, failmessage);
211​ QFETCH(Term, expectedTerm);
212​
213​ AdvancedQueryParser parser;
214​ const auto testTerm = parser.parse(searchInput);
215​ qDebug() << " result term" << testTerm;
216​ qDebug() << "expected term" << expectedTerm;
217​ if (!failmessage.isEmpty()) {
218​ QEXPECT_FAIL("", qPrintable(failmessage), Continue);
219​ }
220​
221​ QCOMPARE(testTerm, expectedTerm);
222​}
223​
224​void AdvancedQueryParserTest::testNestedParentheses_data()
225​{
226​ QTest::addColumn<QString>("searchInput");
227​ QTest::addColumn<Term>("expectedTerm");
228​ QTest::addColumn<QString>("failmessage");
229​
brunsUnsubmitted
Done

remove

bruns: remove
230​ QTest::newRow("a AND b AND c AND d")
231​ << QStringLiteral("a AND b AND c AND d")
brunsUnsubmitted
Done

Remove the searchInput temporary and ...

bruns: Remove the searchInput temporary and ...
232​ << Term{Term::And, QList<Term>{
233​ Term{QString(), QStringLiteral("a"), Term::Contains},
brunsUnsubmitted
Done

... use the QStringLiteral here

bruns: ... use the QStringLiteral here
234​ Term{QString(), QStringLiteral("b"), Term::Contains},
235​ Term{QString(), QStringLiteral("c"), Term::Contains},
brunsUnsubmitted
Done

Term{QString(), QStringLiteral("a"), Term::Contains},

see http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/

bruns: `Term{QString(), QStringLiteral("a"), Term::Contains},` see http://blog.qt.
236​ Term{QString(), QStringLiteral("d"), Term::Contains},
237​ }}
238​ << QString()
239​ ;
240​ QTest::newRow("(a AND b) AND (c OR d)")
241​ << QStringLiteral("(a AND b) AND (c OR d)")
242​ << Term{Term::And, QList<Term>{
243​ Term{QString(), QStringLiteral("a"), Term::Contains},
244​ Term{QString(), QStringLiteral("b"), Term::Contains},
245​ Term{Term::Or, QList<Term>{
246​ Term{QString(), QStringLiteral("c"), Term::Contains},
247​ Term{QString(), QStringLiteral("d"), Term::Contains},
248​ }}
249​ }}
250​ << QString()
251​ ;
252​ QTest::newRow("(a AND (b AND (c AND d)))")
253​ << QStringLiteral("(a AND (b AND (c AND d)))")
brunsUnsubmitted
Done

Just << QString()

bruns: Just `<< QString()`
254​ << Term{Term::And, QList<Term>{
255​ Term{QString(), QStringLiteral("a"), Term::Contains},
michaelhAuthorUnsubmitted
Done

What about this one? Drop it?

michaelh: What about this one? Drop it?
brunsUnsubmitted
Not Done

I think it should stay as it exercises the stack in the query parser

Pseudocode:

s[0] = a, AND
s[1] = b, AND
t = construct (c AND d)
t = Term( s[1].term, t, s[1].op)  
t = Term( s[0].term, t, s[0].op)
bruns: I think it should stay as it exercises the stack in the query parser Pseudocode: ``` s[0] = a…
256​ Term{QString(), QStringLiteral("b"), Term::Contains},
257​ Term{QString(), QStringLiteral("c"), Term::Contains},
258​ Term{QString(), QStringLiteral("d"), Term::Contains},
259​ }}
260​ << QStringLiteral("no optimization")
261​ ;
262​
263​}
264​
brunsUnsubmitted
Done

Adds no information, just use QString() here (and twice below)

bruns: Adds no information, just use QString() here (and twice below)
michaelhAuthorUnsubmitted
Done

This test fails. "no optimization" serves as a flag, that it's expected to fail and as message why. See lines 217-219. That's why I thought of dropping it, because D11826 does not make it pass.

If we drop this test the failmessage column can be removed.

michaelh: This test fails. "no optimization" serves as a flag, that it's expected to fail and as message…
brunsUnsubmitted
Done

Ah, this makes it somewhat complicated ...

So the parsed query is semantically correct, only the term are not merged?
If yes, the best way here is to probably keep this row as is, but also add a check testing for the unoptimized term. The second check should then carry a comment mentioning it is only a fallback.

Anyway, please make the message somewhat more meaningful, e.g. "Fails to optimize for unknown reason, but output is semantically correct".

bruns: Ah, this makes it somewhat complicated ... So the parsed query is semantically correct, only…
265​void AdvancedQueryParserTest::testOptimizedLogic()
266​{
267​ QFETCH(Term, testTerm);
268​ QFETCH(Term, expectedTerm);
269​ qDebug() << " result term" << testTerm;
270​ qDebug() << "expected term" << expectedTerm;
271​
272​ QEXPECT_FAIL("", "no optimization", Continue);
273​ QCOMPARE(testTerm, expectedTerm);
274​}
275​
276​void AdvancedQueryParserTest::testOptimizedLogic_data()
277​{
278​ QTest::addColumn<Term>("testTerm");
279​ QTest::addColumn<Term>("expectedTerm");
280​
281​ // a && b && c && d can be combined into one AND term with 4 subterms
282​ QTest::addRow("a && b && c && d")
283​ << (Term{QString(), QStringLiteral("a"), Term::Contains}
284​ && Term{QString(), QStringLiteral("b"), Term::Contains}
285​ && Term{QString(), QStringLiteral("c"), Term::Contains}
286​ && Term{QString(), QStringLiteral("d"), Term::Contains})
287​ << Term{Term::And, QList<Term>{
288​ Term{QString(), QStringLiteral("a"), Term::Contains},
289​ Term{QString(), QStringLiteral("b"), Term::Contains},
290​ Term{QString(), QStringLiteral("c"), Term::Contains},
brunsUnsubmitted
Done

Use "a && b && c && d", makes grepping in case of failure easier?

bruns: Use "a && b && c && d", makes grepping in case of failure easier?
291​ Term{QString(), QStringLiteral("d"), Term::Contains},
292​ }}
293​ ;
294​
295​ // (a AND b) AND (c OR d) can be merged as (a AND b AND (c OR D)
296​ QTest::addRow("(a && b) && (c || d)")
297​ << ((Term{QString(), QStringLiteral("a"), Term::Contains}
298​ && Term{QString(), QStringLiteral("b"), Term::Contains})
299​ && (Term{QString(), QStringLiteral("c"), Term::Contains}
300​ || Term{QString(), QStringLiteral("d"), Term::Contains}
301​ ))
302​ << Term{Term::And, QList<Term>{
303​ Term{QString(), QStringLiteral("a"), Term::Contains},
304​ Term{QString(), QStringLiteral("b"), Term::Contains},
305​ Term{Term::Or, QList<Term>{
brunsUnsubmitted
Done

dito, "(a && b) && (c || d)"

bruns: dito, "(a && b) && (c || d)"
306​ Term{QString(), QStringLiteral("c"), Term::Contains},
307​ Term{QString(), QStringLiteral("d"), Term::Contains}
308​ }}
309​ }}
310​ ;
311​
312​}
202 313​
203QTEST_MAIN(AdvancedQueryParserTest) 314​QTEST_MAIN(AdvancedQueryParserTest)
204 315​
205#include "advancedqueryparsertest.moc" 316​#include "advancedqueryparsertest.moc"
brunsUnsubmitted
Done

better use a different name, phrase has a special meaning, i.e. Baloo:EngineQuery:Phrase

bruns: better use a different name, phrase has a special meaning, i.e. Baloo:EngineQuery:Phrase