Re: [PR] [SPARK-48386][TESTS] Replace JVM assert with JUnit Assert in tests [spark]

2024-05-22 Thread via GitHub 


LuciferYang commented on PR #46698:
URL: https://github.com/apache/spark/pull/46698#issuecomment-2126089186

   Merged into master for Spark 4.0. Thanks @panbingkun and @dongjoon-hyun ~


-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Re: [PR] [SPARK-48386][TESTS] Replace JVM assert with JUnit Assert in tests [spark]

2024-05-22 Thread via GitHub 


LuciferYang closed pull request #46698: [SPARK-48386][TESTS] Replace JVM assert 
with JUnit Assert in tests
URL: https://github.com/apache/spark/pull/46698


-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Re: [PR] [SPARK-48386][TESTS] Replace JVM assert with JUnit Assert in tests [spark]

2024-05-22 Thread via GitHub 


dongjoon-hyun commented on PR #46698:
URL: https://github.com/apache/spark/pull/46698#issuecomment-2125880898

   Thank you for updating, @panbingkun .


-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Re: [PR] [SPARK-48386][TESTS] Replace JVM assert with JUnit Assert in tests [spark]

2024-05-22 Thread via GitHub 


panbingkun commented on code in PR #46698:
URL: https://github.com/apache/spark/pull/46698#discussion_r1610689898


##
common/utils/src/test/java/org/apache/spark/util/SparkLoggerSuiteBase.java:
##
@@ -127,8 +128,8 @@ public void testBasicLoggerWithException() {
 Pair.of(Level.DEBUG, debugFn),
 Pair.of(Level.TRACE, traceFn)).forEach(pair -> {
   try {
-assert (captureLogOutput(pair.getRight()).matches(
-expectedPatternForBasicMsgWithException(pair.getLeft(;
+Assertions.assertTrue(captureLogOutput(pair.getRight()).matches(

Review Comment:
   Updated.



##
sql/core/src/test/java/test/org/apache/spark/sql/connector/JavaAdvancedDataSourceV2WithV2Filter.java:
##
@@ -78,9 +80,9 @@ public Predicate[] pushPredicates(Predicate[] predicates) {
 
   Predicate[] unsupported = Arrays.stream(predicates).filter(f -> {
 if (f.name().equals(">")) {
-  assert(f.children()[0] instanceof FieldReference);
+  Assertions.assertInstanceOf(FieldReference.class, f.children()[0]);

Review Comment:
   Updated.



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Re: [PR] [SPARK-48386][TESTS] Replace JVM assert with JUnit Assert in tests [spark]

2024-05-22 Thread via GitHub 


dongjoon-hyun commented on code in PR #46698:
URL: https://github.com/apache/spark/pull/46698#discussion_r1610435821


##
sql/core/src/test/java/test/org/apache/spark/sql/connector/JavaAdvancedDataSourceV2WithV2Filter.java:
##
@@ -78,9 +80,9 @@ public Predicate[] pushPredicates(Predicate[] predicates) {
 
   Predicate[] unsupported = Arrays.stream(predicates).filter(f -> {
 if (f.name().equals(">")) {
-  assert(f.children()[0] instanceof FieldReference);
+  Assertions.assertInstanceOf(FieldReference.class, f.children()[0]);

Review Comment:
   ditto. Shall we import `assertInstanceOf`?



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Re: [PR] [SPARK-48386][TESTS] Replace JVM assert with JUnit Assert in tests [spark]

2024-05-22 Thread via GitHub 


dongjoon-hyun commented on code in PR #46698:
URL: https://github.com/apache/spark/pull/46698#discussion_r1610434320


##
common/utils/src/test/java/org/apache/spark/util/SparkLoggerSuiteBase.java:
##
@@ -127,8 +128,8 @@ public void testBasicLoggerWithException() {
 Pair.of(Level.DEBUG, debugFn),
 Pair.of(Level.TRACE, traceFn)).forEach(pair -> {
   try {
-assert (captureLogOutput(pair.getRight()).matches(
-expectedPatternForBasicMsgWithException(pair.getLeft(;
+Assertions.assertTrue(captureLogOutput(pair.getRight()).matches(

Review Comment:
   Why don't we import `assertTrue`?



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Re: [PR] [SPARK-48386][TESTS] Replace JVM assert with JUnit Assert in tests [spark]

2024-05-22 Thread via GitHub 


dongjoon-hyun commented on code in PR #46698:
URL: https://github.com/apache/spark/pull/46698#discussion_r1610434320


##
common/utils/src/test/java/org/apache/spark/util/SparkLoggerSuiteBase.java:
##
@@ -127,8 +128,8 @@ public void testBasicLoggerWithException() {
 Pair.of(Level.DEBUG, debugFn),
 Pair.of(Level.TRACE, traceFn)).forEach(pair -> {
   try {
-assert (captureLogOutput(pair.getRight()).matches(
-expectedPatternForBasicMsgWithException(pair.getLeft(;
+Assertions.assertTrue(captureLogOutput(pair.getRight()).matches(

Review Comment:
   Why don't we import this?



-- 
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: reviews-unsubscr...@spark.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org