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/11 03:11:08 UTC
[GitHub] [solr] madrob opened a new pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
madrob opened a new pull request #510:
URL: https://github.com/apache/solr/pull/510
https://issues.apache.org/jira/browse/SOLR-15908
--
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
[GitHub] [solr] HoustonPutman commented on a change in pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [solr] madrob commented on a change in pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #510:
URL: https://github.com/apache/solr/pull/510#discussion_r783339215
##########
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:
I hope not, but who knows? If it starts failing then we can at least add a comment or some explanation. Otherwise that seems pretty mysterious to me.
--
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
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #510:
URL: https://github.com/apache/solr/pull/510#discussion_r782509849
##########
File path: solr/solr-ref-guide/tools/org/apache/solr/guide/CheckLinksAndAnchors.java
##########
@@ -119,7 +121,7 @@ public static void main(String[] args) throws Exception {
int problems = 0;
if (args.length < 1) {
- System.err.println("usage: CheckLinksAndAnchors <htmldir> [-check-all-relative-links] [-bare-bones]");
+ System.err.println("usage: org.apache.solr.guide.CheckLinksAndAnchors <htmldir> [-check-all-relative-links] [-bare-bones]");
System.exit(-1);
}
final File htmlDir = new File(args[0]);
Review comment:
I've recorded this as ignored for this pull request. If you change your mind, just comment `@sonatype-lift unignore`.
--
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
[GitHub] [solr] HoustonPutman commented on a change in pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
Posted by GitBox <gi...@apache.org>.
HoustonPutman commented on a change in pull request #510:
URL: https://github.com/apache/solr/pull/510#discussion_r783394917
##########
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:
interesting, if we remove one digit, it stays as `0.3333333f`. That would probably be more useful in the sorting test?
--
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
[GitHub] [solr] madrob commented on a change in pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #510:
URL: https://github.com/apache/solr/pull/510#discussion_r783338351
##########
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:
Floats aren't very precise. Sometimes you ask for one thing and get another!
I'm not sure if the value was intentionally equal to the previous value, or if this was an oversight.
```
jshell> 0.33333333f
$1 ==> 0.33333334
```
--
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
[GitHub] [solr] madrob merged pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
Posted by GitBox <gi...@apache.org>.
madrob merged pull request #510:
URL: https://github.com/apache/solr/pull/510
--
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
[GitHub] [solr] sonatype-lift[bot] commented on a change in pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
Posted by GitBox <gi...@apache.org>.
sonatype-lift[bot] commented on a change in pull request #510:
URL: https://github.com/apache/solr/pull/510#discussion_r781766239
##########
File path: solr/solr-ref-guide/tools/org/apache/solr/guide/CheckLinksAndAnchors.java
##########
@@ -119,7 +121,7 @@ public static void main(String[] args) throws Exception {
int problems = 0;
if (args.length < 1) {
- System.err.println("usage: CheckLinksAndAnchors <htmldir> [-check-all-relative-links] [-bare-bones]");
+ System.err.println("usage: org.apache.solr.guide.CheckLinksAndAnchors <htmldir> [-check-all-relative-links] [-bare-bones]");
System.exit(-1);
}
final File htmlDir = new File(args[0]);
Review comment:
*PATH_TRAVERSAL_IN:* This API (java/io/File.<init>(Ljava/lang/String;)V) reads a file whose location might be specified by user input [(details)](https://find-sec-bugs.github.io/bugs.htm#PATH_TRAVERSAL_IN)
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with `help` or `ignore`)
--
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
[GitHub] [solr] madrob commented on a change in pull request #510: SOLR-15908 Document or fix error-prone patterns relating to tests
Posted by GitBox <gi...@apache.org>.
madrob commented on a change in pull request #510:
URL: https://github.com/apache/solr/pull/510#discussion_r782509822
##########
File path: solr/solr-ref-guide/tools/org/apache/solr/guide/CheckLinksAndAnchors.java
##########
@@ -119,7 +121,7 @@ public static void main(String[] args) throws Exception {
int problems = 0;
if (args.length < 1) {
- System.err.println("usage: CheckLinksAndAnchors <htmldir> [-check-all-relative-links] [-bare-bones]");
+ System.err.println("usage: org.apache.solr.guide.CheckLinksAndAnchors <htmldir> [-check-all-relative-links] [-bare-bones]");
System.exit(-1);
}
final File htmlDir = new File(args[0]);
Review comment:
@sonatype-lift ignore
--
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