You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by GitBox <gi...@apache.org> on 2022/04/05 06:12:45 UTC
[GitHub] [druid] Codegass opened a new issue, #12399: Test Cases are using the print instead of Assert in testing
Codegass opened a new issue, #12399:
URL: https://github.com/apache/druid/issues/12399
## Description
When I am running the test for [DataGenerator](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java), I noticed that some test cases in this test class don’t have any Assert from JUnit:
* [testLazyZipf](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L424)
* [testRealUniform](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L334)
* [testNormal](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L291)
* [testEnumerated](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L262)
* [testZipf](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L207)
* [testRoundedNormal](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L164)
* [ testDiscreteUniform ](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L99)
* [testSequential](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L49)
These test cases are all using the [`printStuff`](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L529) function to print out the related information for manual verification. There is no JUnit Assert function in these test cases. Based on my understanding, these cases require manual checking of the printed information to verify whether the test cases pass or fail.
Also, The [testIntervalBasedTimeGeneration()](https://github.com/apache/druid/blob/b86f2d4c2e935346d600e51b22403150ebd1501d/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java#L375) test case does not have print info. It's hard for new contributors to tell whether it passes or fails.
I am curious about why these test cases are designed without Assert. And if adding the assert is helpful, I would be more than happy to try submit a PR.
--
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: commits-unsubscribe@druid.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [I] Test Cases are using the print instead of Assert in testing (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 commented on issue #12399:
URL: https://github.com/apache/druid/issues/12399#issuecomment-1839863087
@AlbericByte Thank you for picking this up. As far as I know, there's no one working on it.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [I] Test Cases are using the print instead of Assert in testing (druid)
Posted by "AlbericByte (via GitHub)" <gi...@apache.org>.
AlbericByte commented on issue #12399:
URL: https://github.com/apache/druid/issues/12399#issuecomment-1839854599
@FrankChen021 i could work on this if no other work on this already.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
[GitHub] [druid] xvrl commented on issue #12399: Test Cases are using the print instead of Assert in testing
Posted by GitBox <gi...@apache.org>.
xvrl commented on issue #12399:
URL: https://github.com/apache/druid/issues/12399#issuecomment-1092195372
Hi @Codegass feel free to submit a PR to improve some of those things. The printing in test cases might be a relic of someone debugging some of these things. To the extent that things have to be checked as part of the test, I agree it's useful to use test assertions instead. For other things that are there for information, we could probably replace them with debug statements instead
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [I] Test Cases are using the print instead of Assert in testing (druid)
Posted by "AlbericByte (via GitHub)" <gi...@apache.org>.
AlbericByte commented on issue #12399:
URL: https://github.com/apache/druid/issues/12399#issuecomment-1843858448
Thanks @FrankChen021 and @xvrl , please help to have a look when you have time.
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org
Re: [I] Test Cases are using the print instead of Assert in testing (druid)
Posted by "FrankChen021 (via GitHub)" <gi...@apache.org>.
FrankChen021 closed issue #12399: Test Cases are using the print instead of Assert in testing
URL: https://github.com/apache/druid/issues/12399
--
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: commits-unsubscribe@druid.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org