dcapwell commented on code in PR #2104:
URL: https://github.com/apache/cassandra/pull/2104#discussion_r1132718796
##########
test/distributed/org/apache/cassandra/distributed/test/streaming/BoundExceptionTest.java:
##########
@@ -34,38 +32,36 @@
@Test
public void testSingleException()
{
- Throwable boundedStackTrace = StreamSession.boundStackTrace(new
RuntimeException("test exception"), LIMIT);
+ Throwable exceptionToTest = new RuntimeException("test exception");
+ StringBuilder boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
String expectedStackTrace = "java.lang.RuntimeException: test
exception\n" +
- "\tat
org.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testSingleException(BoundExceptionTest.java:37)\n"
+
- "\tat
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)\n";
+
"\torg.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testSingleException(BoundExceptionTest.java:35)\n"
+
+
"\tjava.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)";
-
assertEquals(expectedStackTrace,Throwables.getStackTraceAsString(boundedStackTrace));
- assertEquals(boundedStackTrace.getStackTrace().length, LIMIT);
+ assertEquals(expectedStackTrace,boundedStackTrace.toString());
+
assertEquals(boundedStackTrace.toString().split(System.lineSeparator()).length
- 1, LIMIT);
}
@Test
public void testNestedException()
{
Throwable exceptionToTest = new RuntimeException(new
IllegalArgumentException("the disk /foo/var is bad", new IOException("Bad disk
somewhere")));
- Throwable boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
+ StringBuilder boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
String expectedStackTrace1 = "java.lang.RuntimeException:
java.lang.IllegalArgumentException: the disk /foo/var is bad\n" +
- "\tat
org.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testNestedException(BoundExceptionTest.java:50)\n";
- String expectedStackTrace2 = "java.lang.IllegalArgumentException: the
disk /foo/var is bad\n";
- String expectedStackTrace3 = "java.io.IOException: Bad disk
somewhere\n";
-
- String boundedStackTraceAsString =
Throwables.getStackTraceAsString(boundedStackTrace);
-
- assertTrue(boundedStackTraceAsString.contains(expectedStackTrace1));
- assertTrue(boundedStackTraceAsString.contains(expectedStackTrace2));
- assertTrue(boundedStackTraceAsString.contains(expectedStackTrace3));
-
- while (boundedStackTrace != null)
- {
- assertEquals(boundedStackTrace.getStackTrace().length, LIMIT);
- boundedStackTrace = boundedStackTrace.getCause();
- }
+
"\torg.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testNestedException(BoundExceptionTest.java:49)\n"
+
+
"\tjava.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)\n" +
+ "java.lang.IllegalArgumentException: the
disk /foo/var is bad\n" +
+
"\torg.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testNestedException(BoundExceptionTest.java:49)\n"
+
+
"\tjava.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)\n" +
+ "java.io.IOException: Bad disk
somewhere\n" +
+
"\torg.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testNestedException(BoundExceptionTest.java:49)\n"
+
+
"\tjava.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)";
+
+ String boundedStackTraceAsString = boundedStackTrace.toString();
Review Comment:
don't save to a variable if you are not using multiple times
##########
test/distributed/org/apache/cassandra/distributed/test/streaming/BoundExceptionTest.java:
##########
@@ -34,38 +32,36 @@
@Test
public void testSingleException()
{
- Throwable boundedStackTrace = StreamSession.boundStackTrace(new
RuntimeException("test exception"), LIMIT);
+ Throwable exceptionToTest = new RuntimeException("test exception");
+ StringBuilder boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
String expectedStackTrace = "java.lang.RuntimeException: test
exception\n" +
- "\tat
org.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testSingleException(BoundExceptionTest.java:37)\n"
+
- "\tat
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)\n";
+
"\torg.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testSingleException(BoundExceptionTest.java:35)\n"
+
+
"\tjava.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)";
-
assertEquals(expectedStackTrace,Throwables.getStackTraceAsString(boundedStackTrace));
- assertEquals(boundedStackTrace.getStackTrace().length, LIMIT);
+ assertEquals(expectedStackTrace,boundedStackTrace.toString());
+
assertEquals(boundedStackTrace.toString().split(System.lineSeparator()).length
- 1, LIMIT);
Review Comment:
given your string makes sure this is true, do we need this?
##########
test/distributed/org/apache/cassandra/distributed/test/streaming/BoundExceptionTest.java:
##########
@@ -77,19 +73,16 @@ public void testExceptionCycle()
e1.initCause(e2);
e2.initCause(e1);
- Throwable boundedStackTrace = StreamSession.boundStackTrace(e1, LIMIT);
- String expectedStackTrace1 = "java.lang.Exception: Test exception 1\n"
+
- "\tat
org.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testExceptionCycle(BoundExceptionTest.java:74)\n";
- String expectedStackTrace2 = "java.lang.RuntimeException: Test
exception 2\n" +
- "\tat
org.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testExceptionCycle(BoundExceptionTest.java:75)\n";
- String expectedStackTrace3 = "[CIRCULAR REFERENCE:
java.lang.Exception: Test exception 1]\n";
-
- String boundedStackTraceAsString =
Throwables.getStackTraceAsString(boundedStackTrace);
+ StringBuilder boundedStackTrace = StreamSession.boundStackTrace(e1,
LIMIT);
+ String expectedStackTrace = "java.lang.Exception: Test exception 1\n" +
+
"\torg.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testExceptionCycle(BoundExceptionTest.java:70)\n"
+
+
"\tjava.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)\n" +
+ "java.lang.RuntimeException: Test
exception 2\n" +
+
"\torg.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testExceptionCycle(BoundExceptionTest.java:71)\n"
+
+
"\tjava.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method)\n" +
+ "[CIRCULAR REFERENCE:
java.lang.Exception: Test exception 1]";
- assertTrue(boundedStackTraceAsString.contains(expectedStackTrace1));
- assertTrue(boundedStackTraceAsString.contains(expectedStackTrace2));
- assertTrue(boundedStackTraceAsString.contains(expectedStackTrace3));
- assertEquals(boundedStackTrace.getStackTrace().length, LIMIT);
+ assertEquals(expectedStackTrace, boundedStackTrace.toString());
}
@Test
Review Comment:
you are missing a test where stack element length < limit, you have a bug in
this case
##########
test/distributed/org/apache/cassandra/distributed/test/streaming/BoundExceptionTest.java:
##########
@@ -98,46 +91,26 @@ public void testEmptyStackTrace()
Throwable exceptionToTest = new NullPointerException("there are words
here");
exceptionToTest.setStackTrace(new StackTraceElement[0]);
- Throwable boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
+ StringBuilder boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
String expectedStackTrace = "java.lang.NullPointerException: there are
words here\n";
-
assertEquals(expectedStackTrace,Throwables.getStackTraceAsString(boundedStackTrace));
- assertEquals(boundedStackTrace.getStackTrace().length, 0);
- }
-
- @Test
- public void testLimitLargerThanStackTrace()
- {
- Throwable exceptionToTest = new NullPointerException("there are words
here");
- Throwable boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
- Throwable reboundedStackTrace =
StreamSession.boundStackTrace(boundedStackTrace, LIMIT + 2);
-
- assertEquals(reboundedStackTrace, boundedStackTrace);
- assertEquals(reboundedStackTrace.getStackTrace().length, LIMIT);
+ assertEquals(expectedStackTrace,boundedStackTrace.toString());
}
@Test
public void testEmptyNestedStackTrace()
{
Throwable exceptionToTest = new RuntimeException(new
IllegalArgumentException("the disk /foo/var is bad", new IOException("Bad disk
somewhere")));
-
exceptionToTest.setStackTrace(new StackTraceElement[0]);
- exceptionToTest.getCause().setStackTrace(new StackTraceElement[0]);
exceptionToTest.getCause().getCause().setStackTrace(new
StackTraceElement[0]);
- Throwable boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
+ StringBuilder boundedStackTrace =
StreamSession.boundStackTrace(exceptionToTest, LIMIT);
String expectedStackTrace = "java.lang.RuntimeException:
java.lang.IllegalArgumentException: the disk /foo/var is bad\n" +
- "Caused by:
java.lang.IllegalArgumentException: the disk /foo/var is bad\n" +
- "Caused by: java.io.IOException: Bad disk
somewhere\n";
-
- String boundedStackTraceAsString =
Throwables.getStackTraceAsString(boundedStackTrace);
-
- assertEquals(boundedStackTraceAsString, expectedStackTrace);
+ "java.lang.IllegalArgumentException: the
disk /foo/var is bad\n" +
+
"\torg.apache.cassandra.distributed.test.streaming.BoundExceptionTest.testEmptyNestedStackTrace(BoundExceptionTest.java:103)\n"
+
Review Comment:
I am ok with this semantic if that is what you feel is best, as we only
print 2 elements... left a comment in another test that you are printing more
than 2 frames and that middle causes are less important.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
