Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update to picocli 3.9.1 result in UT MainTest.testNonExistentOutputFormat failure #6397

Closed
romani opened this Issue Jan 29, 2019 · 7 comments

Comments

Projects
None yet
3 participants
@romani
Copy link
Member

romani commented Jan 29, 2019

detected in #6382.

Test:
https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/MainTest.java#L290

In debug it is clear that exit code is -1 as required.
But for some reason junit consider test as failed.
On new picocli and old picocli there is exception throw at https://github.com/stefanbirkner/system-rules/blob/master/src/main/java/org/junit/contrib/java/lang/system/internal/NoExitSecurityManager.java#L24

but on new version of picocli it is considered as failure.

Full UT failure stacktrace:

org.junit.contrib.java.lang.system.internal.CheckExitCalled: Tried to exit with status -1.

	at org.junit.contrib.java.lang.system.internal.NoExitSecurityManager.checkExit(NoExitSecurityManager.java:24)
	at java.lang.Runtime.exit(Runtime.java:107)
	at java.lang.System.exit(System.java:971)
	at com.puppycrawl.tools.checkstyle.Main.main(Main.java:143)
	at com.puppycrawl.tools.checkstyle.MainTest.testNonExistentOutputFormat(MainTest.java:298)
	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.internal.runners.TestMethod.invoke(TestMethod.java:68)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:326)
	at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:89)
	at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:97)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.executeTest(PowerMockJUnit44RunnerDelegateImpl.java:310)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTestInSuper(PowerMockJUnit47RunnerDelegateImpl.java:131)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.access$100(PowerMockJUnit47RunnerDelegateImpl.java:59)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner$TestExecutorStatement.evaluate(PowerMockJUnit47RunnerDelegateImpl.java:147)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
	at org.junit.contrib.java.lang.system.ExpectedSystemExit$1.evaluate(ExpectedSystemExit.java:130)
	at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:48)
	at org.junit.contrib.java.lang.system.internal.LogPrintStream$1$1.evaluate(LogPrintStream.java:30)
	at org.junit.contrib.java.lang.system.internal.PrintStreamHandler$3.evaluate(PrintStreamHandler.java:48)
	at org.junit.contrib.java.lang.system.internal.LogPrintStream$1.evaluate(LogPrintStream.java:26)
	at org.junit.contrib.java.lang.system.internal.LogPrintStream$1$1.evaluate(LogPrintStream.java:30)
	at org.junit.contrib.java.lang.system.internal.PrintStreamHandler$3.evaluate(PrintStreamHandler.java:48)
	at org.junit.contrib.java.lang.system.internal.LogPrintStream$1.evaluate(LogPrintStream.java:26)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.evaluateStatement(PowerMockJUnit47RunnerDelegateImpl.java:107)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit47RunnerDelegateImpl$PowerMockJUnit47MethodRunner.executeTest(PowerMockJUnit47RunnerDelegateImpl.java:82)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$PowerMockJUnit44MethodRunner.runBeforesThenTestThenAfters(PowerMockJUnit44RunnerDelegateImpl.java:298)
	at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:87)
	at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:50)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.invokeTestMethod(PowerMockJUnit44RunnerDelegateImpl.java:218)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.runMethods(PowerMockJUnit44RunnerDelegateImpl.java:160)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl$1.run(PowerMockJUnit44RunnerDelegateImpl.java:134)
	at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:34)
	at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:44)
	at org.powermock.modules.junit4.internal.impl.PowerMockJUnit44RunnerDelegateImpl.run(PowerMockJUnit44RunnerDelegateImpl.java:136)
	at org.powermock.modules.junit4.common.internal.impl.JUnit4TestSuiteChunkerImpl.run(JUnit4TestSuiteChunkerImpl.java:117)
	at org.powermock.modules.junit4.common.internal.impl.AbstractCommonPowerMockRunner.run(AbstractCommonPowerMockRunner.java:57)
	at org.powermock.modules.junit4.PowerMockRunner.run(PowerMockRunner.java:59)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)

@romani romani added the approved label Jan 29, 2019

@romani

This comment has been minimized.

Copy link
Member Author

romani commented Jan 30, 2019

@remkop , do you have any ideas why there is such regression from 3.9.0 to 3.9.1 ?
I see smth in release notes about Systems.exit() - https://github.com/remkop/picocli/releases#user-content-3.9.1-fixes

@remkop

This comment has been minimized.

Copy link
Contributor

remkop commented Jan 30, 2019

Yes, I also had trouble with the tests that check the exit code and some other condition (for example the error message string). The test would fail saying “wrong exit code”, while actually the exit code is fine but it’s the other condition that’s failing (for example the error message string is different from the expected value).

Looking at the test, I suspect the test fails because the error message string for incorrect enum values changed slightly in picocli 3.9.1.

Probably related to this change:
592 Error message now shows enum constant names, not toString() values, after value mismatch

@rnveach

This comment has been minimized.

Copy link
Member

rnveach commented Jan 30, 2019

The test would fail saying “wrong exit code”, while actually the exit code is fine but it’s the other condition that’s failing (for example the error message string is different from the expected value).

This is just a guess, but this may be happening because it checks the output before the system exit code. Since there is a message mis-match, it throws an assertion exception and then treats that as the system exit instead of populating the assertion failure to the junit runner to the user.

@remkop

This comment has been minimized.

Copy link
Contributor

remkop commented Feb 19, 2019

FYI, if you consider upgrading, please upgrade to picocli 3.9.5.
https://github.com/remkop/picocli/releases/tag/v3.9.5

This upgrade is important: native code included in jansi-1.14 seems to have a bug that can crash the JVM on Linux. Picocli does not include jansi, but jansi is included in Gradle 4.5.x and I've seen this manifest on continuous integration servers running tests from Gradle.

(Version details: RHEL 3.10.0-327.44.2.el7.x86_64 on Java 1.8.0_112-b15, Java HotSpot(TM) 64-Bit Server VM (build 25.112-b15, mixed mode)).

Picocli 3.9.5 will only load jansi classes when running on Windows. Picocli versions 3.9.0 to 3.9.4 may load jansi classes when running on non-Windows platforms and are vulnerable to this problem.

@rnveach

This comment has been minimized.

Copy link
Member

rnveach commented Feb 19, 2019

@romani @remkop See reason for weird junit failure at #6438 (comment) .
Short version is powermock is mangling the AssertionError and making it seem like it is an exit code failure instead. With powermock removed, AssertionError comes through correctly and you can see the change in the output.

rnveach added a commit to rnveach/checkstyle that referenced this issue Feb 19, 2019

romani added a commit that referenced this issue Feb 19, 2019

@romani romani added this to the 8.18 milestone Feb 19, 2019

@romani

This comment has been minimized.

Copy link
Member Author

romani commented Feb 19, 2019

Fix is merged

@romani romani closed this Feb 19, 2019

@remkop

This comment has been minimized.

Copy link
Contributor

remkop commented Feb 20, 2019

Thank you @romani @rnveach !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.