bq. " Having this elsewhere in the code encourages it to creep in more."
+1. While I hesitate to make lots of changes that are unnecessary, the
other side of that argument is that when we see code its easy to think
it's the norm rather than old....
On Mon, Jul 9, 2018 at 4:52 AM, Jason Gerlowski <gerlowsk...@gmail.com> wrote:
> I authored the recent change you're commenting on. I agree with your
> points; my only defense is consistency. Several other nearby
> assertions used the older try-catch based setup.
>
> I'll fix the spot you objected to, and file a JIRA around cleaning
> this up more broadly. Having this elsewhere in the code encourages it
> to creep in more.
>
> Best,
>
> Jason
> On Fri, Jul 6, 2018 at 12:58 PM Chris Hostetter
> <hossman_luc...@fucit.org> wrote:
>>
>>
>> these tests should really be using...
>>
>> SolrException e = expectThrows(() -> {...});
>>
>> ...and ideally we should be making assertions about the exception message
>> as well (ie: does it say what we expect it to say? does it give the user
>> the context of the failure -- ie: containing the "non_numeric_value" so
>> they know what they did wrong?
>>
>>
>> : private void validateCommonQueryParameters() throws Exception {
>> : ignoreException("parameter cannot be negative");
>> : +
>> : + try {
>> : + SolrQuery query = new SolrQuery();
>> : + query.setParam("start", "non_numeric_value").setQuery("*");
>> : + QueryResponse resp = query(query);
>> : + fail("Expected the last query to fail, but got response: " + resp);
>> : + } catch (SolrException e) {
>> : + assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
>> : + }
>> : +
>> : try {
>> : SolrQuery query = new SolrQuery();
>> : query.setStart(-1).setQuery("*");
>> : @@ -1228,6 +1238,15 @@ public class TestDistributedSearch extends
>> BaseDistributedSearchTestCase {
>> : } catch (SolrException e) {
>> : assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
>> : }
>> : +
>> : + try {
>> : + SolrQuery query = new SolrQuery();
>> : + query.setParam("rows", "non_numeric_value").setQuery("*");
>> : + QueryResponse resp = query(query);
>> : + fail("Expected the last query to fail, but got response: " + resp);
>> : + } catch (SolrException e) {
>> : + assertEquals(ErrorCode.BAD_REQUEST.code, e.code());
>> : + }
>> : resetExceptionIgnores();
>> : }
>> : }
>> :
>> :
>>
>> -Hoss
>> http://www.lucidworks.com/
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>> For additional commands, e-mail: dev-h...@lucene.apache.org
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
> For additional commands, e-mail: dev-h...@lucene.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
