You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@solr.apache.org by GitBox <gi...@apache.org> on 2022/01/12 17:56:59 UTC
[GitHub] [solr] HoustonPutman commented on a change in pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
HoustonPutman commented on a change in pull request #510:
URL: https://github.com/apache/solr/pull/510#discussion_r783314160
##########
File path: solr/core/src/test/org/apache/solr/update/TestExceedMaxTermLength.java
##########
@@ -72,23 +72,18 @@ public void testExceededMaxTermLength(){
assertFailedU(doc);
} else {
//Use JSON
- try {
- if(includeOkayFields) {
- String jsonStr = "[{'id':'1','%s':'%s', '%s': '%s'}]";
- jsonStr = String.format(Locale.ROOT, jsonStr, longFieldName, longFieldValue,
- okayFieldName, okayFieldValue);
- updateJ(json(jsonStr), null);
- } else {
- String jsonStr = "[{'id':'1','%s':'%s'}]";
- jsonStr = String.format(Locale.ROOT, jsonStr, longFieldName, longFieldValue);
- updateJ(json(jsonStr), null);
- }
- } catch (Exception e) {
- //expected
- String msg= e.getCause().getMessage();
- assertTrue(msg.contains("one immense term in field=\"cat\""));
+ final String jsonStr;
+ if(includeOkayFields) {
+ String format = "[{'id':'1','%s':'%s', '%s': '%s'}]";
+ jsonStr = String.format(Locale.ROOT, format, longFieldName, longFieldValue,
+ okayFieldName, okayFieldValue);
+ } else {
+ String format = "[{'id':'1','%s':'%s'}]";
+ jsonStr = String.format(Locale.ROOT, format, longFieldName, longFieldValue);
}
-
+ Exception e = assertThrows(Exception.class, () -> updateJ(json(jsonStr), null));
+ String msg = e.getCause().getMessage();
Review comment:
I would probably assertNotNull(e.getCause()) here as well.
##########
File path: solr/test-framework/src/java/org/apache/solr/cloud/api/collections/AbstractIncrementalBackupTest.java
##########
@@ -510,11 +510,7 @@ private void assertFolderAreSame(URI uri1, URI uri2) throws IOException {
Arrays.sort(files1);
Arrays.sort(files2);
- try {
- assertArrayEquals(files1, files2);
- } catch (AssertionError e) {
- e.printStackTrace();
- }
+ assertArrayEquals(files1, files2);
Review comment:
Does this fail occasionally?
##########
File path: solr/core/src/test/org/apache/solr/handler/export/TestExportWriter.java
##########
@@ -684,13 +684,13 @@ public void testMultipleSorts() throws Exception {
float_vals[5] = Float.MAX_VALUE;
float_vals[6] = Float.MIN_VALUE;
float_vals[7] = 1/3f; //0.33333334
- float_vals[8] = 0.33333333f;
+ float_vals[8] = 0.33333333f; //0.33333334, intentional?
Review comment:
what's this comment for?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@solr.apache.org
For additional commands, e-mail: issues-help@solr.apache.org