[
https://issues.apache.org/jira/browse/CASSANDRA-16630?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17444372#comment-17444372
]
Aleksei Zotov edited comment on CASSANDRA-16630 at 11/23/21, 12:16 PM:
-----------------------------------------------------------------------
[~e.dimitrova]
I'd like to share an update on the current progress. I got a working prototype
with JUnit 5 ([https://github.com/apache/cassandra/pull/1337]; whatever exists
in "junitlauncher2" package will be updated in ant project and removed from the
PR). It is still pending full blown validation, but I was able to run one test
:)
Looks like currently _ant-junitlauncher_ (ant JUnit5 implementation) misses
many features available in {_}ant-junit{_}. That's why I raised two more PRs
to the ant project:
* [https://github.com/apache/ant/pull/168] (completed)
* [https://github.com/apache/ant/pull/169] (it is a draft version since I need
to validate that all use cases are handled before passing it for review)
With that being said, I do not believe we can get all this work done soon
because of dependencies to another project. JFYI.
Now regarding the validation I've done so far. I was able to check xml
formatter only. Here is the list of differences I still have in the output
(compared old and new implementation):
1. Different way of closing of "testcase" tag:
{code:java}
<testcase classname="org.apache.cassandra.Junit4SampleTest" name="testSuccess"
time="0.001" /> {code}
{code:java}
<testcase classname="org.apache.cassandra.Junit4SampleTest" name="testSuccess"
time="0.01"></testcase> {code}
It not an issue since the format is still valid.
2. "errors" attribute in "testsuite" tag was renamed to "aborted":
{code:java}
<testsuite name="org.apache.cassandra.Junit4SampleTest" time="0.082"
timestamp="2021-11-13T19:45:32" tests="2" failures="1" skipped="0" errors="0"
hostname="azotov-hp"> {code}
{code:java}
<testsuite name="org.apache.cassandra.Junit4SampleTest" time="0.117"
timestamp="2021-11-15T13:54:00" tests="2" failures="1" skipped="0" aborted="0"
hostname="azotov-hp"> {code}
Theoretically it can be fixed, but I do not think it is required.
3. New line escape character got changed from " " to "
":
{code:java}
<property name="rat.failed.files" value="Unapproved
licenses:


*******************************

Archives (+
indicates readable, $ unreadable): " /> {code}
{code:java}
<property name="rat.failed.files" value="Unapproved
licenses:&#xa;&#xa;&#xa;*******************************&#xa;&#xa;Archives
(+ indicates readable, $ unreadable): "></property>
{code}
In fact it is related to the internal implementation changes of DOM and Stax
XML parsers. I did not dig really deep since it does not seem to be a problem
at all.
4. "failure" tag body now contains non-filtered stacktrace:
{code:java}
<failure message="Expected failure"
type="junit.framework.AssertionFailedError">junit.framework.AssertionFailedError:
Expected failure
at
org.apache.cassandra.Junit4SampleTest.testFailure(Junit4SampleTest.java:37)
</failure> {code}
{code:java}
<failure message="Expected failure"
type="java.lang.AssertionError">java.lang.AssertionError: Expected failure
at org.junit.Assert.fail(Assert.java:89) at
org.apache.cassandra.Junit4SampleTest.testFailure(Junit4SampleTest.java:39)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498) at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at
org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366) at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) at
org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) at
org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) at
org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) at
org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) at
org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at
org.junit.runners.ParentRunner.run(ParentRunner.java:413) at
org.junit.runner.JUnitCore.run(JUnitCore.java:137) at
org.junit.runner.JUnitCore.run(JUnitCore.java:115) at
org.junit.vintage.engine.execution.RunnerExecutor.execute(RunnerExecutor.java:43)
at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
at
java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.Iterator.forEachRemaining(Iterator.java:116) at
java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
at
java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
at
java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
at
java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
at
org.junit.vintage.engine.VintageTestEngine.executeAllChildren(VintageTestEngine.java:82)
at
org.junit.vintage.engine.VintageTestEngine.execute(VintageTestEngine.java:73)
at
org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:170)
at
org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:154)
at
org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:90)
at
org.apache.tools.ant.taskdefs.optional.junitlauncher.LauncherSupport.launch(LauncherSupport.java:144)
at
org.apache.tools.ant.taskdefs.optional.junitlauncher.StandaloneLauncher.main(StandaloneLauncher.java:114)
</failure>
{code}
The newer _ant-junitlauncher_ version does not have necessary filtering
capabilities. I gave a try in writing this logic manually, but it seems to be
clunky and not very accurate. I can invest more time, but I'm not sure that
full traces is a problem since we do not expect many failed tests, so it won't
pollute our logs.
5. "system-out" and "system-err" tags are not being printed if there is no
system out / system err:
{code:java}
<testsuite ...>
<properties/>
<testcase/>
<system-out><![CDATA[]]></system-out>
<system-err><![CDATA[]]></system-err>
</testsuite> {code}
{code:java}
<testsuite ...>
<properties/>
<testcase/>
</testsuite>
{code}
I was able to to make logic working the same way as previously, however, I
don't feel it is required because having "system-out" and "system-err" tags
without data makes no much sense. So I propose to stick to the new logic.
6. "failure" body is now a CData instead of plain text:
{code:java}
<failure message="Expected failure"
type="junit.framework.AssertionFailedError">junit.framework.AssertionFailedError:
Expected failure at
org.apache.cassandra.Junit4SampleTest.testFailure(Junit4SampleTest.java:37)
</failure> {code}
{code:java}
<failure message="Expected failure"
type="java.lang.AssertionError"><![CDATA[java.lang.AssertionError: Expected
failure
at org.junit.Assert.fail(Assert.java:89)]]>
</failure>
{code}
I fixed this difference (and you can see in point #4), however, I like having
CData more (it seems to be more correct approach). In general I'd like to
minimize the number of customizations. So I propose to use the new format with
CData here.
[~e.dimitrova] [~mck] Please, review the above output diffs for the XML
formatter and let me know if you believe any of the changes may affect current
pipeline (I'd be very grateful if you could comment on every point). Even
though I'm able to fix all diffs (some would be hard but doable) and keep
exactly the same format as for JUnit 4, I do want to minimize customizations
and stick to the default JUnit 5 format where possible.
I the meantime, I'll proceed with Brief formatter testing and try to finalize
the second _ant_ PR.
was (Author: azotcsit):
[~e.dimitrova]
I'd like to share an update on the current progress. I got a working prototype
with JUnit 5 ([https://github.com/apache/cassandra/pull/1322/]; whatever exists
in "junitlauncher2" package will be updated in ant project and removed from the
PR). It is still pending full blown validation, but I was able to run one test
:)
Looks like currently _ant-junitlauncher_ (ant JUnit5 implementation) misses
many features available in {_}ant-junit{_}. That's why I raised two more PRs
to the ant project:
* [https://github.com/apache/ant/pull/168] (completed)
* [https://github.com/apache/ant/pull/169] (it is a draft version since I need
to validate that all use cases are handled before passing it for review)
With that being said, I do not believe we can get all this work done soon
because of dependencies to another project. JFYI.
Now regarding the validation I've done so far. I was able to check xml
formatter only. Here is the list of differences I still have in the output
(compared old and new implementation):
1. Different way of closing of "testcase" tag:
{code:java}
<testcase classname="org.apache.cassandra.Junit4SampleTest" name="testSuccess"
time="0.001" /> {code}
{code:java}
<testcase classname="org.apache.cassandra.Junit4SampleTest" name="testSuccess"
time="0.01"></testcase> {code}
It not an issue since the format is still valid.
2. "errors" attribute in "testsuite" tag was renamed to "aborted":
{code:java}
<testsuite name="org.apache.cassandra.Junit4SampleTest" time="0.082"
timestamp="2021-11-13T19:45:32" tests="2" failures="1" skipped="0" errors="0"
hostname="azotov-hp"> {code}
{code:java}
<testsuite name="org.apache.cassandra.Junit4SampleTest" time="0.117"
timestamp="2021-11-15T13:54:00" tests="2" failures="1" skipped="0" aborted="0"
hostname="azotov-hp"> {code}
Theoretically it can be fixed, but I do not think it is required.
3. New line escape character got changed from "
" to "&#xa;":
{code:java}
<property name="rat.failed.files" value="Unapproved
licenses:


*******************************

Archives (+
indicates readable, $ unreadable): " /> {code}
{code:java}
<property name="rat.failed.files" value="Unapproved
licenses:&#xa;&#xa;&#xa;*******************************&#xa;&#xa;Archives
(+ indicates readable, $ unreadable): "></property>
{code}
In fact it is related to the internal implementation changes of DOM and Stax
XML parsers. I did not dig really deep since it does not seem to be a problem
at all.
4. "failure" tag body now contains non-filtered stacktrace:
{code:java}
<failure message="Expected failure"
type="junit.framework.AssertionFailedError">junit.framework.AssertionFailedError:
Expected failure
at
org.apache.cassandra.Junit4SampleTest.testFailure(Junit4SampleTest.java:37)
</failure> {code}
{code:java}
<failure message="Expected failure"
type="java.lang.AssertionError">java.lang.AssertionError: Expected failure
at org.junit.Assert.fail(Assert.java:89) at
org.apache.cassandra.Junit4SampleTest.testFailure(Junit4SampleTest.java:39)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at
sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
at
sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498) at
org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at
org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at
org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at
org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at
org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366) at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at
org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331) at
org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79) at
org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329) at
org.junit.runners.ParentRunner.access$100(ParentRunner.java:66) at
org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293) at
org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306) at
org.junit.runners.ParentRunner.run(ParentRunner.java:413) at
org.junit.runner.JUnitCore.run(JUnitCore.java:137) at
org.junit.runner.JUnitCore.run(JUnitCore.java:115) at
org.junit.vintage.engine.execution.RunnerExecutor.execute(RunnerExecutor.java:43)
at java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
at
java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:193)
at java.util.Iterator.forEachRemaining(Iterator.java:116) at
java.util.Spliterators$IteratorSpliterator.forEachRemaining(Spliterators.java:1801)
at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:482)
at
java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:472)
at
java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
at
java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:418)
at
org.junit.vintage.engine.VintageTestEngine.executeAllChildren(VintageTestEngine.java:82)
at
org.junit.vintage.engine.VintageTestEngine.execute(VintageTestEngine.java:73)
at
org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:170)
at
org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:154)
at
org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:90)
at
org.apache.tools.ant.taskdefs.optional.junitlauncher.LauncherSupport.launch(LauncherSupport.java:144)
at
org.apache.tools.ant.taskdefs.optional.junitlauncher.StandaloneLauncher.main(StandaloneLauncher.java:114)
</failure>
{code}
The newer _ant-junitlauncher_ version does not have necessary filtering
capabilities. I gave a try in writing this logic manually, but it seems to be
clunky and not very accurate. I can invest more time, but I'm not sure that
full traces is a problem since we do not expect many failed tests, so it won't
pollute our logs.
5. "system-out" and "system-err" tags are not being printed if there is no
system out / system err:
{code:java}
<testsuite ...>
<properties/>
<testcase/>
<system-out><![CDATA[]]></system-out>
<system-err><![CDATA[]]></system-err>
</testsuite> {code}
{code:java}
<testsuite ...>
<properties/>
<testcase/>
</testsuite>
{code}
I was able to to make logic working the same way as previously, however, I
don't feel it is required because having "system-out" and "system-err" tags
without data makes no much sense. So I propose to stick to the new logic.
6. "failure" body is now a CData instead of plain text:
{code:java}
<failure message="Expected failure"
type="junit.framework.AssertionFailedError">junit.framework.AssertionFailedError:
Expected failure at
org.apache.cassandra.Junit4SampleTest.testFailure(Junit4SampleTest.java:37)
</failure> {code}
{code:java}
<failure message="Expected failure"
type="java.lang.AssertionError"><![CDATA[java.lang.AssertionError: Expected
failure
at org.junit.Assert.fail(Assert.java:89)]]>
</failure>
{code}
I fixed this difference (and you can see in point #4), however, I like having
CData more (it seems to be more correct approach). In general I'd like to
minimize the number of customizations. So I propose to use the new format with
CData here.
[~e.dimitrova] [~mck] Please, review the above output diffs for the XML
formatter and let me know if you believe any of the changes may affect current
pipeline (I'd be very grateful if you could comment on every point). Even
though I'm able to fix all diffs (some would be hard but doable) and keep
exactly the same format as for JUnit 4, I do want to minimize customizations
and stick to the default JUnit 5 format where possible.
I the meantime, I'll proceed with Brief formatter testing and try to finalize
the second _ant_ PR.
> Migrate to JUnit5
> -----------------
>
> Key: CASSANDRA-16630
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16630
> Project: Cassandra
> Issue Type: Improvement
> Components: Test/unit
> Reporter: Aleksei Zotov
> Assignee: Aleksei Zotov
> Priority: Low
> Time Spent: 10m
> Remaining Estimate: 0h
>
> h3. Overview
> Currently C* uses JUnit4 (version 4.12) which is obsolete. There is a newer
> version 4.13.2 which we could update to. However, JUnit4 is generally
> considered to be outdated and it is reasonable to migrate to JUnit5.
> Despite of having a syntax sugar in JUnit5 (assertThrow, lamda's support,
> ect), there are no blockers that push us to move from JUnit4. The main
> motivation for this initiative is rule of thumb to use up-to-date versions of
> the dependencies.
> Obviously this change is not backward compatible with the open PRs and
> previous C* versions. Therefore, it will require an additional effort for
> backporting the changes and updating PRs that have tests. However, I believe
> it should not be a blocker for this initiative.
> h3. Scope (preliminary list)
> # change JUnit4 to JUnit5 dependencies and make necessary changes in ant
> tasks (https://ant.apache.org/manual/Tasks/junitlauncher.html)
> # update syntax in all tests (imports, Before/After annotations, etc)
> # update parameterized tests
> # create a new version of {{OrderedJUnit4ClassRunner}} and update
> corresponding tests
> # update tests that use {{BMUnitRunner}} (as per
> https://developer.jboss.org/docs/DOC-52953 it supports JUnit5)
> # update tests with {{@Rule}}
> # update tests with expected exceptions
> # update {{JStackJUnitTask}}
> # update formatters
> # create a separate ticket to migrate to {{ant-junitlauncher-1.10.11}} (once
> it is released) and simplify {{JStackJUnitTask}} after
> https://github.com/apache/ant/pull/147
> h3. Order of operations
> In order to make the transition more smooth we want to use a phased approach:
> # migrate to JUnit5 with [Vintage
> Engine|https://junit.org/junit5/docs/current/user-guide/#dependency-metadata-junit-vintage],
> so all JUnit4 tests work as is
> # update tests in a few bunches (to not have a huge single PR with numerous
> conflicts)
> # disable (remove dependency) Vintage Engine, so only JUnit5 tests work
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
