sbabcoc / JUnit-Foundation

JUnit Foundation is a lightweight collection of JUnit watchers, interfaces, and static utility classes that supplement and augment the functionality provided by the JUnit API.
Apache License 2.0
22 stars 6 forks source link

JUnit Foundation does not fire any listeners in case of PowerMock usage (@RunWith(PowerMockRunner.class)) #77

Closed HardNorth closed 3 years ago

HardNorth commented 3 years ago

Hello, When someone uses a PowerMock runner in tests JUnit Foundation does not fire any events in listeners.

E.G.:

import org.apache.commons.lang3.RandomStringUtils;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.equalTo;
import static org.mockito.Mockito.when;
import static org.powermock.api.mockito.PowerMockito.mockStatic;

@RunWith(PowerMockRunner.class)
@PrepareForTest(StaticClass.class)
public class SimplePowermockTest {
    @Test
    public void testPowerMock() {
        String rndStr = RandomStringUtils.randomAlphanumeric(10);
        mockStatic(StaticClass.class);
        when(StaticClass.staticMethod()).thenReturn(rndStr);
        assertThat(StaticClass.staticMethod(), equalTo(rndStr));
    }
}

I've prepared two Test Cases to reproduce the issue:

And here are corresponding tests, which catch the issue (currently disabled):

sbabcoc commented 3 years ago

@HardNorth - The Java agent specification in the agent-java-junit build file should be revised to match the example shown in the README. Also, there's no need to override the transitive dependency of JUnit Foundation 12.3.0, as it already includes JUnit 4.13.1.

One thing that confuses me completely is the specification of JUnit Jupiter references (JUnit 5) in testImplementation entries of the agent-java-junit build file. This agent is very specifically targeted at JUnit 4. I don't know that this version mismatch is causing problems, but it certainly looks wrong.

HardNorth commented 3 years ago

@sbabcoc Which exact example do you mean and why agent build file should match a test project example file? These are two separate things. I agree we need to revise our examples since they are too old-fashioned. But this is a separate task.

As for the dependency it is here because all JUnit 4 agent tests were written on JUnit 5. Since it is testImplementation dependency it does not affect the agent by any way. I've also implicitly excluded any conflict dependencies. The idea behind the agent tests is to launch real JUnit 4 tests and verify requests which were passed to RP. I can't write everything on JUnit 4 since in that case JUnit Foundation will capture these tests and fire listener events messing them with test examples. The approach of such testing is already well-developed and well-tested, every agent of version 5 for RP which was released so far (eight of them) is tested in the same manner.

So I'm pretty sure agent code is not the case of the issue. I can provide a simple example project, which will not include our agent at all if you like.

sbabcoc commented 3 years ago

@HardNorth - Sorry, I'm referring to the Gradle project file example. The agent specification for JUnit Foundation has been greatly simplified and avoids some issues that were caused by the prior formulation.

Referring to exclusion of JUnit transitive dependency, this is unnecessary and potentially problematic. JUnit Foundation works by installing hooks into JUnit 4 core classes via runtime code generation. This is performed by the Java agent and only affects test flows that are executed by these dynamically modified classes. If any of the classes that need to be hooked get loaded prior to the execution of the JUnit Foundation agent, the unmodified versions of these classes will be used instead. This means that the associated test lifecycle events will not fire.

You should be able to trace the execution of your test methods through JUnit Foundation. Set a breakpoint on RunAnnouncer.testStarted(Description) and debug an affected PowerMock test to see if this ever gets called.

HardNorth commented 3 years ago

I'm actually already removed dependency exclusion. And I'm going to update readme file soon.

Ok. I'll post you back with the investigation details.

sbabcoc commented 3 years ago

@HardNorth - I added PowerMock unit tests to the JUnit Foundation project, and it works as expected. The tests are declared as JUnit 4 @Test methods in a class that gets executed by a JUnitCore runner created by a TestNG @Test method in a different class.

I strongly suspect that the trouble you're experiencing is caused by the inclusion of the JUnit 5 API into your project to execute the JUnit 4 tests. This is either preventing JUnit Foundation from installing altered versions of core JUnit 4 classes or bypassing the actual JUnit 4 classes altogether.

Another possibility is that you're getting tripped up by using the static JUnitCore.runClasses(Class<?>...) method, which creates a Suite runner on the fly to execute your classes. This would affect all of your tests, though... not just the PowerMock tests.

HardNorth commented 3 years ago

@sbabcoc, OK, got you. Please give me few days on the next week, I need to investigate the case.

HardNorth commented 3 years ago

@sbabcoc Hello again. I've prepared a very simple example project with two tests (one with PowerMock and one without): https://github.com/HardNorth/junit-powermock-check

If i run it with: ./gradlew test The result will be:

JUnitTestPowerMock > testPowerMock FAILED
    java.lang.AssertionError at JUnitTestPowerMock.java:23

2 tests completed, 1 failed

The assert which fails indicates that a test with PowerMock fire no events in a dummy listener.

So, the issue not in dependencies. Since there is almost no dependencies in this project.

sbabcoc commented 3 years ago

@HardNorth - Sorry for the inaccurate feedback. I swear I got this running, but I had a few errors in my execution. I now have a unit test the reproduces the issue, and the core challenge is the PowerMock is using a runner model that has been deprecated since JUnit 4.5 - which was released more than 12 years ago! This is a completely different API than JUnit Foundation was designed to support, and it would take a tremendous amount of effort to add handling for this deprecated mechanism for running tests. There was actually a span of 6 years where these deprecated classes were pulled out of JUnit, but they were added back in to restore backward compatibility.

HardNorth commented 3 years ago

@sbabcoc NP, I understand your troubles. I just need a proper reason to my users. If no - than no. The reason seems legit to me. It's for you to decide whether it worth to support.

sbabcoc commented 3 years ago

@HardNorth - There may be a way to do this via the @PowerMockRunnerDelegate annotation. I'm investigating this now.

sbabcoc commented 3 years ago

Try adding this to your test class: @PowerMockRunnerDelegate(BlockJUnit4ClassRunner.class)

sbabcoc commented 3 years ago

@HardNorth - This change gets things to start working, but execution blows up with a very confusing configuration error in the Settings API. I'll see if I can dig into this a bit more in the next few days to see if I can fix this configuration failure.

sbabcoc commented 3 years ago

@HardNorth - OK, I found an easy fix... I'll have a new build posted later today.

sbabcoc commented 3 years ago

@HardNorth - New release 12.5.0 has been published. Pay special attention to the NOTE and code example in the release description regarding the @PowerMockRunnerDelegate annotation.

Also, the verification in your example project still doesn't work, but this is due to a threading issue in your implementation. Your RUNNER_COUNTER needs to be thread-local (specifically InheritableThreadLocal) to avoid unintended instantiation of a new counter in the new thread spawned by PowerMockRunner.

HardNorth commented 3 years ago

Duplicating here our conversation: @sbabcoc For PowerMock I have no success. So I wanted to clarify, what exact I need to update in tests? Just add @PowerMockRunnerDelegate(BlockJUnit4ClassRunner.class) ?

I did that in my tests: https://github.com/reportportal/agent-java-junit/commit/d17757e2394bb8ac79eb2f057e2c30a38f414891 But they fail if I run them :(

I’m seeing this in my console:

WARNING: An illegal reflective access operation has occurred
WARNING: Illegal reflective access by org.powermock.reflect.internal.WhiteboxImpl (file:/Users/vadzim_hushchanskou/.gradle/caches/modules-2/files-2.1/org.powermock/powermock-reflect/2.0.9/4bb9ed43e5221926fb86cae44b445de110a51d05/powermock-reflect-2.0.9.jar) to method java.lang.Object.clone()
WARNING: Please consider reporting this to the maintainers of org.powermock.reflect.internal.WhiteboxImpl
WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations
WARNING: All illegal access operations will be denied in a future release
12:51:39.943 DEBUG c.n.a.j.Run - main - runFinished: org.junit.runners.BlockJUnit4ClassRunner@37df14d1
Exception in thread "main" java.lang.Error: org.apache.commons.configuration2.ex.ConfigurationRuntimeException: Incompatible result object: org.apache.commons.configuration2.PropertiesConfiguration@62b790a5
    at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner.run(DelegatingPowerMockRunner.java:158)
    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:69)
    at com.intellij.rt.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:33)
    at com.intellij.rt.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:220)
    at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)
Caused by: org.apache.commons.configuration2.ex.ConfigurationRuntimeException: Incompatible result object: org.apache.commons.configuration2.PropertiesConfiguration@62b790a5
    at org.apache.commons.configuration2.builder.BasicConfigurationBuilder.checkResultInstance(BasicConfigurationBuilder.java:728)
    at org.apache.commons.configuration2.builder.BasicConfigurationBuilder.createResultInstance(BasicConfigurationBuilder.java:448)
    at org.apache.commons.configuration2.builder.BasicConfigurationBuilder.createResult(BasicConfigurationBuilder.java:417)
    at org.apache.commons.configuration2.builder.BasicConfigurationBuilder.getConfiguration(BasicConfigurationBuilder.java:285)
    at org.apache.commons.configuration2.builder.fluent.Configurations.properties(Configurations.java:318)
    at com.nordstrom.automation.settings.SettingsCore.<init>(SettingsCore.java:136)
    at com.nordstrom.automation.junit.JUnitConfig.<init>(JUnitConfig.java:73)
    at com.nordstrom.automation.junit.JUnitConfig$1.initialValue(JUnitConfig.java:59)
    at com.nordstrom.automation.junit.JUnitConfig$1.initialValue(JUnitConfig.java:55)
    at java.base/java.lang.ThreadLocal.setInitialValue(ThreadLocal.java:195)
    at java.base/java.lang.ThreadLocal.get(ThreadLocal.java:172)
    at com.nordstrom.automation.junit.JUnitConfig.getConfig(JUnitConfig.java:82)
    at com.nordstrom.automation.junit.Run.fireRunStarted(Run.java:141)
    at com.nordstrom.automation.junit.Run.intercept(Run.java:57)
    at org.junit.runners.BlockJUnit4ClassRunner.run(BlockJUnit4ClassRunner.java)
    at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner$2.call(DelegatingPowerMockRunner.java:149)
    at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner$2.call(DelegatingPowerMockRunner.java:141)
    at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner.withContextClassLoader(DelegatingPowerMockRunner.java:132)
    at org.powermock.modules.junit4.internal.impl.DelegatingPowerMockRunner.run(DelegatingPowerMockRunner.java:141)
    ... 8 more
Process finished with exit code 1