SOLR-12555: Use `expectThrows` for expected exceptions

This commit replaces the `try { doX(); fail(); }` pattern with the
`expectThrows` test helper, which was created for this purpose.

This commit makes these changes in the core package: `o.a.solr.search`.

Closes #464
This commit is contained in:
Jason Gerlowski 2018-11-02 17:12:41 -04:00
parent c5ff4a4444
commit f669a1fb0e
10 changed files with 147 additions and 229 deletions

View File

@ -1202,14 +1202,8 @@ public class QueryEqualityTest extends SolrTestCaseJ4 {
assertFuncEquals("gte(foo_i,2)", "gte(foo_i,2)");
assertFuncEquals("eq(foo_i,2)", "eq(foo_i,2)");
boolean equals = false;
try {
assertFuncEquals("eq(foo_i,2)", "lt(foo_i,2)");
equals = true;
} catch (AssertionError e) {
//expected
}
assertFalse(equals);
expectThrows(AssertionError.class, "expected error, functions are not equal",
() -> assertFuncEquals("eq(foo_i,2)", "lt(foo_i,2)"));
}
public void testChildField() throws Exception {
@ -1223,32 +1217,25 @@ public class QueryEqualityTest extends SolrTestCaseJ4 {
}
public void testPayloadScoreQuery() throws Exception {
// I don't see a precedent to test query inequality in here, so doing a `try`
// There was a bug with PayloadScoreQuery's .equals() method that said two queries were equal with different includeSpanScore settings
try {
assertQueryEquals
("payload_score"
, "{!payload_score f=foo_dpf v=query func=min includeSpanScore=false}"
, "{!payload_score f=foo_dpf v=query func=min includeSpanScore=true}"
);
fail("queries should not have been equal");
} catch(AssertionFailedError e) {
assertTrue("queries were not equal, as expected", true);
}
expectThrows(AssertionFailedError.class, "queries should not have been equal",
() -> assertQueryEquals
("payload_score"
, "{!payload_score f=foo_dpf v=query func=min includeSpanScore=false}"
, "{!payload_score f=foo_dpf v=query func=min includeSpanScore=true}"
)
);
}
public void testPayloadCheckQuery() throws Exception {
try {
assertQueryEquals
("payload_check"
, "{!payload_check f=foo_dpf payloads=2}one"
, "{!payload_check f=foo_dpf payloads=2}two"
);
fail("queries should not have been equal");
} catch(AssertionFailedError e) {
assertTrue("queries were not equal, as expected", true);
}
expectThrows(AssertionFailedError.class, "queries should not have been equal",
() -> assertQueryEquals
("payload_check"
, "{!payload_check f=foo_dpf payloads=2}one"
, "{!payload_check f=foo_dpf payloads=2}two"
)
);
}
public void testPayloadFunction() throws Exception {
@ -1272,16 +1259,14 @@ public class QueryEqualityTest extends SolrTestCaseJ4 {
"must='{!lucene}foo_s:c' filter='{!lucene}foo_s:d' filter='{!lucene}foo_s:e'}",
"{!bool must='{!lucene}foo_s:c' filter='{!lucene}foo_s:d' " +
"must_not='{!lucene}foo_s:a' should='{!lucene}foo_s:b' filter='{!lucene}foo_s:e'}");
try {
assertQueryEquals
("bool"
, "{!bool must='{!lucene}foo_s:a'}"
, "{!bool should='{!lucene}foo_s:a'}"
);
fail("queries should not have been equal");
} catch(AssertionFailedError e) {
assertTrue("queries were not equal, as expected", true);
}
expectThrows(AssertionFailedError.class, "queries should not have been equal",
() -> assertQueryEquals
("bool"
, "{!bool must='{!lucene}foo_s:a'}"
, "{!bool should='{!lucene}foo_s:a'}"
)
);
}
// Override req to add df param

View File

@ -881,13 +881,10 @@ public class TestCollapseQParserPlugin extends SolrTestCaseJ4 {
public void testGroupHeadSelector() {
GroupHeadSelector s;
try {
s = GroupHeadSelector.build(params("sort", "foo_s asc", "min", "bar_s"));
fail("no exception with multi criteria");
} catch (SolrException e) {
// expected
}
expectThrows(SolrException.class, "no exception with multi criteria",
() -> GroupHeadSelector.build(params("sort", "foo_s asc", "min", "bar_s"))
);
s = GroupHeadSelector.build(params("min", "foo_s"));
assertEquals(GroupHeadSelectorType.MIN, s.type);

View File

@ -656,45 +656,35 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
public void testCyclicAliasing() throws Exception {
try {
ignoreException(".*Field aliases lead to a cycle.*");
try {
h.query(req("defType","edismax", "q","blarg", "qf","who", "f.who.qf","name","f.name.qf","who"));
fail("Simple cyclic alising not detected");
} catch (SolrException e) {
assertTrue(e.getCause().getMessage().contains("Field aliases lead to a cycle"));
}
try {
h.query(req("defType","edismax", "q","blarg", "qf","who", "f.who.qf","name","f.name.qf","myalias", "f.myalias.qf","who"));
fail("Cyclic alising not detected");
} catch (SolrException e) {
assertTrue(e.getCause().getMessage().contains("Field aliases lead to a cycle"));
}
try {
h.query(req("defType","edismax", "q","blarg", "qf","field1", "f.field1.qf","field2 field3","f.field2.qf","field4 field5", "f.field4.qf","field5", "f.field5.qf","field6", "f.field3.qf","field6"));
} catch (SolrException e) {
assertFalse("This is not cyclic alising", e.getCause().getMessage().contains("Field aliases lead to a cycle"));
assertTrue(e.getCause().getMessage().contains("not a valid field name"));
}
try {
h.query(req("defType","edismax", "q","blarg", "qf","field1", "f.field1.qf","field2 field3", "f.field2.qf","field4 field5", "f.field4.qf","field5", "f.field5.qf","field4"));
fail("Cyclic alising not detected");
} catch (SolrException e) {
assertTrue(e.getCause().getMessage().contains("Field aliases lead to a cycle"));
}
try {
h.query(req("defType","edismax", "q","who:(Zapp Pig)", "qf","text", "f.who.qf","name","f.name.qf","myalias", "f.myalias.qf","who"));
fail("Cyclic alising not detected");
} catch (SolrException e) {
assertTrue(e.getCause().getMessage().contains("Field aliases lead to a cycle"));
}
SolrException e = expectThrows(SolrException.class, "Simple cyclic alising not detected",
() -> h.query(req("defType","edismax", "q","blarg", "qf","who", "f.who.qf","name","f.name.qf","who")));
assertCyclicDetectionErrorMessage(e);
e = expectThrows(SolrException.class, "Cyclic alising not detected",
() -> h.query(req("defType","edismax", "q","blarg", "qf","who", "f.who.qf","name","f.name.qf","myalias", "f.myalias.qf","who")));
assertCyclicDetectionErrorMessage(e);
e = expectThrows(SolrException.class, "Cyclic aliasing not detected", () -> h.query(req("defType","edismax", "q","blarg", "qf","field1", "f.field1.qf","field2 field3","f.field2.qf","field4 field5", "f.field4.qf","field5", "f.field5.qf","field6", "f.field3.qf","field6")));
assertFalse("This is not cyclic aliasing", e.getCause().getMessage().contains("Field aliases lead to a cycle"));
assertTrue("Should throw exception due to invalid field name", e.getCause().getMessage().contains("not a valid field name"));
e = expectThrows(SolrException.class, "Cyclic alising not detected",
() -> h.query(req("defType","edismax", "q","blarg", "qf","field1", "f.field1.qf","field2 field3", "f.field2.qf","field4 field5", "f.field4.qf","field5", "f.field5.qf","field4")));
assertCyclicDetectionErrorMessage(e);
e = expectThrows(SolrException.class, "Cyclic alising not detected",
() -> h.query(req("defType","edismax", "q","who:(Zapp Pig)", "qf","text", "f.who.qf","name","f.name.qf","myalias", "f.myalias.qf","who")));
assertCyclicDetectionErrorMessage(e);
} finally {
resetExceptionIgnores();
}
}
private void assertCyclicDetectionErrorMessage(SolrException e) {
assertTrue(e.getCause().getMessage().contains("Field aliases lead to a cycle"));
}
public void testOperatorsWithLiteralColons() {
assertU(adoc("id", "142", "a_s", "bogus:xxx", "text_s", "yak"));
assertU(adoc("id", "143", "a_s", "bogus:xxx"));
@ -2092,8 +2082,8 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
public void killInfiniteRecursionParse() throws Exception {
assertJQ(req("defType", "edismax", "q", "*", "qq", "{!edismax v=something}", "bq", "{!edismax v=$qq}"));
}
/** SOLR-5163 */
/** SOLR-5163 */
@Test
public void testValidateQueryFields() throws Exception {
// field aliasing covered by test - testAliasing
@ -2103,13 +2093,13 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
params.add("q", "olive AND other");
params.add("qf", "subject^3 title");
params.add("debugQuery", "true");
// test valid field names
try (SolrQueryRequest req = req(params)) {
String response = h.query(req);
response.contains("+DisjunctionMaxQuery((title:olive | (subject:oliv)^3.0)) +DisjunctionMaxQuery((title:other | (subject:other)^3.0))");
}
// test invalid field name
params.set("qf", "subject^3 nosuchfield");
try (SolrQueryRequest req = req(params)) {
@ -2117,7 +2107,7 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
} catch (Exception e) {
Assert.assertEquals("org.apache.solr.search.SyntaxError: Query Field 'nosuchfield' is not a valid field name", e.getMessage());
}
}
}

View File

@ -304,9 +304,9 @@ public class TestFoldingMultitermQuery extends SolrTestCaseJ4 {
public void testMultiBad() {
try {
ignoreException("analyzer returned too many terms");
assertQ(req("q", "content_multi_bad:" + "abCD*"));
fail("Should throw exception when token evaluates to more than one term");
} catch (Exception expected) {
Exception expected = expectThrows(Exception.class, "Should throw exception when token evaluates to more than one term",
() -> assertQ(req("q", "content_multi_bad:" + "abCD*"))
);
assertTrue(expected.getCause() instanceof org.apache.solr.common.SolrException);
} finally {
resetExceptionIgnores();

View File

@ -182,11 +182,8 @@ public class TestLRUCache extends LuceneTestCase {
CacheRegenerator cr = new NoOpRegenerator();
Object o = cache.init(params, null, cr);
try {
cache.put("1", "1");
fail("Adding a non-accountable value to a cache configured with maxRamBytes should have failed");
} catch (Exception e) {
assertEquals(e.getClass(), SolrException.class);
}
expectThrows(SolrException.class, "Adding a non-accountable value to a cache configured with maxRamBytes should have failed",
() -> cache.put("1", "1")
);
}
}

View File

@ -389,17 +389,17 @@ public class TestQueryTypes extends SolrTestCaseJ4 {
try {
ignoreException("No\\ default\\, and no switch case");
assertQ("no match and no default",
RuntimeException exp = expectThrows(RuntimeException.class, "Should have gotten an error w/o default",
() -> assertQ("no match and no default",
req("q", "{!switch case.x=Dude case.z=Yonik}asdf")
,"//result[@numFound='BOGUS']");
fail("Should have gotten an error w/o default");
} catch (RuntimeException exp) {
assertTrue("exp cause is wrong",
exp.getCause() instanceof SolrException);
, "//result[@numFound='BOGUS']")
);
assertTrue("exp cause is wrong",
exp.getCause() instanceof SolrException);
SolrException e = (SolrException) exp.getCause();
assertEquals("error isn't user error", 400, e.code());
assertTrue("Error doesn't include bad switch case: " + e.getMessage(),
e.getMessage().contains("asdf"));
e.getMessage().contains("asdf"));
} finally {
resetExceptionIgnores();
}

View File

@ -598,12 +598,11 @@ public class TestReRankQParserPlugin extends SolrTestCaseJ4 {
params.add("start", "0");
params.add("rows", "2");
try {
h.query(req(params));
fail("A syntax error should be thrown when "+ReRankQParserPlugin.RERANK_QUERY+" parameter is not specified");
} catch (SolrException e) {
assertTrue(e.code() == SolrException.ErrorCode.BAD_REQUEST.code);
}
SolrException se = expectThrows(SolrException.class, "A syntax error should be thrown when "+ReRankQParserPlugin.RERANK_QUERY+" parameter is not specified",
() -> h.query(req(params))
);
assertTrue(se.code() == SolrException.ErrorCode.BAD_REQUEST.code);
}
@Test

View File

@ -335,76 +335,53 @@ public class TestRealTimeGet extends TestRTGBase {
clearIndex();
assertU(commit());
long version = addAndGetVersion(sdoc("id","1") , null);
final long version = addAndGetVersion(sdoc("id","1") , null);
long version2;
try {
// try version added directly on doc
version2 = addAndGetVersion(sdoc("id","1", "_version_", Long.toString(version-1)), null);
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// try version added directly on doc
SolrException se = expectThrows(SolrException.class, "version should cause an error",
() -> addAndGetVersion(sdoc("id","1", "_version_", Long.toString(version-1)), null));
assertEquals("version should cause a conflict", 409, se.code());
try {
// try version added as a parameter on the request
version2 = addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(version-1)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// try version added as a parameter on the request
se = expectThrows(SolrException.class, "version should cause an error",
() -> addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(version-1))));
assertEquals("version should cause a conflict", 409, se.code());
try {
// try an add specifying a negative version
version2 = addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(-version)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// try an add specifying a negative version
se = expectThrows(SolrException.class, "negative version should cause a conflict",
() -> addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(-version))));
assertEquals("version should cause a conflict", 409, se.code());
try {
// try an add with a greater version
version2 = addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(version+random().nextInt(1000)+1)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// try an add with a greater version
se = expectThrows(SolrException.class, "greater version should cause a conflict",
() -> addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(version+random().nextInt(1000)+1))));
assertEquals("version should cause a conflict", 409, se.code());
//
// deletes
//
try {
// try a delete with version on the request
version2 = deleteAndGetVersion("1", params("_version_", Long.toString(version-1)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// try a delete with version on the request
se = expectThrows(SolrException.class, "version should cause an error",
() -> deleteAndGetVersion("1", params("_version_", Long.toString(version-1))));
assertEquals("version should cause a conflict", 409, se.code());
try {
// try a delete with a negative version
version2 = deleteAndGetVersion("1", params("_version_", Long.toString(-version)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// try a delete with a negative version
se = expectThrows(SolrException.class, "negative version should cause an error",
() -> deleteAndGetVersion("1", params("_version_", Long.toString(-version))));
assertEquals("version should cause a conflict", 409, se.code());
try {
// try a delete with a greater version
version2 = deleteAndGetVersion("1", params("_version_", Long.toString(version+random().nextInt(1000)+1)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// try a delete with a greater version
se = expectThrows(SolrException.class, "greater version should cause an error",
() -> deleteAndGetVersion("1", params("_version_", Long.toString(version+random().nextInt(1000)+1))));
assertEquals("version should cause a conflict", 409, se.code());
// try a delete of a document that doesn't exist, specifying a specific version
se = expectThrows(SolrException.class, "document does not exist should cause an error",
() -> deleteAndGetVersion("I_do_not_exist", params("_version_", Long.toString(version))));
assertEquals("version should cause a conflict", 409, se.code());
try {
// try a delete of a document that doesn't exist, specifying a specific version
version2 = deleteAndGetVersion("I_do_not_exist", params("_version_", Long.toString(version)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// try a delete of a document that doesn't exist, specifying that it should not
version2 = deleteAndGetVersion("I_do_not_exist", params("_version_", Long.toString(-1)));
@ -414,56 +391,44 @@ public class TestRealTimeGet extends TestRTGBase {
version2 = addAndGetVersion(sdoc("id","1", "_version_", Long.toString(version)), null);
assertTrue(version2 > version);
try {
// overwriting the previous version should now fail
version2 = addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(version)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// overwriting the previous version should now fail
se = expectThrows(SolrException.class, "overwriting previous version should fail",
() -> addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(version))));
assertEquals(409, se.code());
try {
// deleting the previous version should now fail
version2 = deleteAndGetVersion("1", params("_version_", Long.toString(version)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// deleting the previous version should now fail
se = expectThrows(SolrException.class, "deleting the previous version should now fail",
() -> deleteAndGetVersion("1", params("_version_", Long.toString(version))));
assertEquals(409, se.code());
version = version2;
final long prevVersion = version2;
// deleting the current version should work
version2 = deleteAndGetVersion("1", params("_version_", Long.toString(version)));
version2 = deleteAndGetVersion("1", params("_version_", Long.toString(prevVersion)));
try {
// overwriting the previous existing doc should now fail (since it was deleted)
version2 = addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(version)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// overwriting the previous existing doc should now fail (since it was deleted)
se = expectThrows(SolrException.class, "overwriting the previous existing doc should now fail (since it was deleted)",
() -> addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(prevVersion))));
assertEquals(409, se.code());
try {
// deleting the previous existing doc should now fail (since it was deleted)
version2 = deleteAndGetVersion("1", params("_version_", Long.toString(version)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
// deleting the previous existing doc should now fail (since it was deleted)
se = expectThrows(SolrException.class, "deleting the previous existing doc should now fail (since it was deleted)",
() -> deleteAndGetVersion("1", params("_version_", Long.toString(prevVersion))));
assertEquals(409, se.code());
// overwriting a negative version should work
version2 = addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(-(version-1))));
version2 = addAndGetVersion(sdoc("id","1"), params("_version_", Long.toString(-(prevVersion-1))));
assertTrue(version2 > version);
version = version2;
long lastVersion = version2;
// sanity test that we see the right version via rtg
assertJQ(req("qt","/get","id","1")
,"=={'doc':{'id':'1','_version_':" + version + "}}"
,"=={'doc':{'id':'1','_version_':" + lastVersion + "}}"
);
}
/***
/***
@Test
public void testGetRealtime() throws Exception {
SolrQueryRequest sr1 = req("q","foo");
@ -626,12 +591,9 @@ public class TestRealTimeGet extends TestRTGBase {
if (correct) {
version = deleteAndGetVersion(Integer.toString(id), params("_version_", Long.toString(info.version)));
} else {
try {
version = deleteAndGetVersion(Integer.toString(id), params("_version_", Long.toString(badVersion)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
SolrException se = expectThrows(SolrException.class, "should not get random version",
() -> deleteAndGetVersion(Integer.toString(id), params("_version_", Long.toString(badVersion))));
assertEquals(409, se.code());
}
} else {
version = deleteAndGetVersion(Integer.toString(id), null);
@ -674,12 +636,9 @@ public class TestRealTimeGet extends TestRTGBase {
if (correct) {
version = addAndGetVersion(sd, params("_version_", Long.toString(info.version)));
} else {
try {
version = addAndGetVersion(sd, params("_version_", Long.toString(badVersion)));
fail();
} catch (SolrException se) {
assertEquals(409, se.code());
}
SolrException se = expectThrows(SolrException.class, "should not get bad version",
() -> addAndGetVersion(sd, params("_version_", Long.toString(badVersion))));
assertEquals(409, se.code());
}
} else {
version = addAndGetVersion(sd, null);

View File

@ -83,12 +83,10 @@ public class TestSolr4Spatial extends SolrTestCaseJ4 {
"fq", "{!field f=" + fieldName + "}Intersectssss"), 400);
ignoreException("NonexistentShape");
try {
assertU(adoc("id", "-1", fieldName, "NonexistentShape"));
fail();
} catch (SolrException e) {
assertEquals(400, e.code());
}
SolrException e = expectThrows(SolrException.class, "should throw exception on non existent shape",
() -> assertU(adoc("id", "-1", fieldName, "NonexistentShape"))
);
assertEquals(400, e.code());
unIgnoreException("NonexistentShape");
}

View File

@ -353,13 +353,10 @@ public class TestSolrQueryParser extends SolrTestCaseJ4 {
String q = sb.toString();
// This will still fail when used as the main query, but will pass in a filter query since TermsQuery can be used.
try {
{
ignoreException("Too many clauses");
assertJQ(req("q",q)
,"/response/numFound==6");
fail();
} catch (Exception e) {
// expect "too many clauses" exception... see SOLR-10921
SolrException e = expectThrows(SolrException.class, "exoected too many clauses exception",
() -> assertJQ(req("q", q), "/response/numFound==6"));
assertTrue(e.getMessage().contains("many clauses"));
}
@ -1114,13 +1111,9 @@ public class TestSolrQueryParser extends SolrTestCaseJ4 {
for (String suffix:fieldSuffix) {
qParser = QParser.getParser("foo_" + suffix + ":(1 2 3 4 5 6 7 8 9 10 20 19 18 17 16 15 14 13 12 NOT_A_NUMBER)", req);
qParser.setIsFilter(true); // this may change in the future
try {
qParser.getQuery();
fail("Expecting exception");
} catch (SolrException e) {
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue("Unexpected exception: " + e.getMessage(), e.getMessage().contains("Invalid Number: NOT_A_NUMBER"));
}
SolrException e = expectThrows(SolrException.class, "Expecting exception", qParser::getQuery);
assertEquals(SolrException.ErrorCode.BAD_REQUEST.code, e.code());
assertTrue("Unexpected exception: " + e.getMessage(), e.getMessage().contains("Invalid Number: NOT_A_NUMBER"));
}