The UpgradeTest job refuses to pass on PR #5011. As far I can tell,
changing a few tests from UnitTest to IntegrationTest should NOT cause
UpgradeTest to fail. It's the Tomcat Session upgrade tests and they just
seem to fail every single time in this PR. Again, dunno what to do with
this. Anyone familiar with the Tomcat Session upgrade tests want to take a
look?

On Mon, Apr 27, 2020 at 4:25 PM Kirk Lund <kl...@pivotal.io> wrote:

> The problems in WindowsUnitTest jobs in CI are caused by various unit
> tests that are either bugged or should be moved to IntegrationTests.
>
> I found 3 tests named *IntegrationTests in src/test that need to move:
>
> * 
> geode-core/src/test/java/org/apache/geode/distributed/internal/InternalDistributedSystemIntegrationTest.java
>
> * 
> geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/OqlQueryRequestOperationHandlerIntegrationTest.java
>
> * 
> geode-wan/src/test/java/org/apache/geode/internal/cache/wan/GatewaySenderEventRemoteDispatcherIntegrationTest.java
>
> These are unit tests that are better off renamed and moved to
> src/integrationTest because they touch so many Geode classes including
> singletons:
>
> * 
> geode-core/src/test/java/org/apache/geode/distributed/internal/InternalLocatorTest.java
>
> * 
> geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceJUnitTest.java
>
> This one creates a full DistributedSystem stack so it belongs in
> src/integrationTest:
>
> * 
> geode-core/src/test/java/org/apache/geode/distributed/internal/InternalDistributedSystemTest.java
>
> And these unit tests create a full Cache stack so they belong in
> src/integrationTest:
>
> * 
> geode-lucene/src/test/java/org/apache/geode/cache/lucene/FlatFormatPdxSerializerJunitTest.java
>
> * 
> geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/serialization/codec/JsonPdxConverterJUnitTest.java
>
> And one last unit test that I was able to fix -- this test creates a spy
> to test as a partial mock with a common goof that results in a full Cache
> being created and then not used by the test:
>
> * 
> extensions/geode-modules/src/test/java/org/apache/geode/modules/util/BootstrappingFunctionTest.java
>
> I filed a PR to fix all of the above issues:
> https://github.com/apache/geode/pull/5011
>
> Thanks,
> Kirk
>
> On Mon, Apr 27, 2020 at 1:58 PM Kirk Lund <kl...@pivotal.io> wrote:
>
>> This test started failing consistently on Windows (5 builds in a row so
>> far!):
>>
>> org.apache.geode.internal.net.SocketCreatorFactoryJUnitTest >
>> testNewSSLConfigSSLComponentLocator FAILED
>>     java.lang.AssertionError
>>         at org.junit.Assert.fail(Assert.java:86)
>>         at org.junit.Assert.assertTrue(Assert.java:41)
>>         at org.junit.Assert.assertTrue(Assert.java:52)
>>         at
>> org.apache.geode.internal.net.SocketCreatorFactoryJUnitTest.testNewSSLConfigSSLComponentLocator(SocketCreatorFactoryJUnitTest.java:106)
>>
>> The method testNewSSLConfigSSLComponentLocator is the first
>> in SocketCreatorFactoryJUnitTest to execute. And because a previous unit
>> test initialized the singleton either directly or indirectly without
>> cleaning it up in tearDown, it pollutes the JVM for later tests. The only
>> later unit test that is affected seems to be SocketCreatorFactoryJUnitTest.
>>
>> Now you could easily fix SocketCreatorFactoryJUnitTest by adding a new
>> setUp() method:
>>
>>
>> *@Before*
>> *public void setUp() throws Exception {*
>> *  SocketCreatorFactory.close();*
>> *}*
>>
>> @After
>> public void tearDown() throws Exception {
>>   SocketCreatorFactory.close();
>> }
>>
>> *This change will fix this specific symptom, but not the underlying
>> problem -- which is "previous unit test initialized SocketCreatorFactory
>> without cleaning it up". *
>>
>> *You can see why it's a problem if a unit test fails to cleanup
>> EVERYTHING that it setup. And you can why singletons (especially internal
>> non-User API singletons) are so dangerous for maintaining a clean GREEN CI.*
>>
>> Cheers,
>> Kirk
>>
>>

Reply via email to