Github user gerlowskija commented on a diff in the pull request:
https://github.com/apache/lucene-solr/pull/425#discussion_r204596268
--- Diff: solr/core/src/test/org/apache/solr/cloud/BasicZkTest.java ---
@@ -150,15 +150,13 @@ public void testBasic() throws Exception {
zkController.getZkClient().setData("/configs/conf1/solrconfig.xml",
new byte[0], true);
// we set the solrconfig to nothing, so this reload should fail
- try {
+ SolrException e = expectThrows(SolrException.class, () -> {
ignoreException("solrconfig.xml");
h.getCoreContainer().reload(h.getCore().getName());
- fail("The reloaded SolrCore did not pick up configs from zookeeper");
--- End diff --
One downside I've noticed in moving towards the `expectThrows` pattern is
that we lose a lot of the nice error messages that would appear if no exception
was thrown at all.
To help with this, I added another version of `expectThrows`, which takes
an additional argument: a String message to display if no exception was thrown.
(Link to that new method here:
https://github.com/apache/lucene-solr/blob/master/lucene/test-framework/src/java/org/apache/lucene/util/LuceneTestCase.java#L2672)
If you're revising this PR (I noticed you marked it as WIP), consider using
that version of `expectThrows` where it makes sense, to preserve the better
error messages.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
