MovingBlocks / Terasology

Terasology - open source voxel world
http://terasology.org
Apache License 2.0
3.68k stars 1.34k forks source link

Help fix broken / ignored unit tests or other analytics! #3859

Open Cervator opened 4 years ago

Cervator commented 4 years ago

After merging #3856 one unit test class, TypeRegistryTest, intermittently stalled during local testing. It may be fine on some OSes or in Jenkins, but oddly it stalled even with the @Ignored annotation added to the tests. It would be good if a few different people to re-enable those tests and run them locally several times (one at a time, all three at once, etc) and make sure they don't stall - if not then we can re-enable and I'll just blame my workspace.

Additionally at present we skip 7 tests in EntityAwareWorldProviderTest and another 7 in ParallelTest for reasons of the past I don't remember off the top of my head, but the files and Git / GitHub history may help shed some background info on that topic.

Finally there are two tests in the ModuleTestingEnvironment that are getting stuck in infinite loops for me testing locally in LocalChunkProvider.checkForUnload() - not sure that'd relate to these changes. ClientConnectionTest and ExampleTest

Beyond unit tests there are plenty of other code analytics that result in issues, although the stats are still only published in our new Jenkins which isn't exposed publicly yet, although regular contributors can request access any time for some insight. In the meantime you can run gradlew check locally and find an abundance of warnings :-)

In Jenkins specifically unit tests tend to fail if they involve a game environment, #3415 has the details and it would be a lovely thing to fix as well.

Marking Good First Issue as these topics are all mildly unrelated to anything else and decent detective hunts for curious contributors, even if they aren't necessarily going to be easy!

Edit: there is also a weird issue where SpotBugs works for some modules but not others, resulting in no build/reports/spotbugs directory. I saw an error related to it at one point, but forgot where. It showed both locally and in Jenkins so it shouldn't be hard to replicate if looking across a few modules. Pretty sure Pathfinding had the failure. Could run gradlew spotbugsMain to selectively just run SpotBugs in a workspace then go looking for which modules made that build dir or not.

Cervator commented 4 years ago

Wanted to leave this link here as well: https://github.com/jenkinsci/warnings-ng-plugin/blob/master/doc/Documentation.md

It goes a bit beyond the origin of this issue, but beyond hooking analytics back up in the new Jenkins at all there are so many options floating around for how we could do more. I also tried to enable the Sonarsource web scanner but while it worked fine for one project dynamically making it figure out multiple from one set of config / add more as needed without manual action might take a bit more work

lizzyd710 commented 4 years ago

How's the code coverage? I've worked with JUnit and Mockito before and would be happy to work with unit tests.

Cervator commented 4 years ago

Hi @lizzyd710! ... well, we have some! 😁

But 700+ tests barely make a dent in the engine project here when it comes to percentage coverage, then there are oodles of modules and other side projects. We're always low on tests - any assistance writing more would be really appreciated!

lizzyd710 commented 4 years ago

In TypeRegistryTest I uncommented the @Test annotations (but not the @Ignored) and none of them stalled for me. Here's my results:

In ParallelTest, all tests except testRunningSuccess pass. For tests that are ignored/disabled but pass when I run them, should I just delete the @Ignored/@Disabled?

Cervator commented 4 years ago

Sure, at this point I'm less worried about the tests stalling in our Jenkins server, since that'll get tested as part of a PR now. Previously when I ignored things and wrote this issue we were still trying to get our build server up and running properly :-)

pree-T commented 2 years ago

Is this issue still open?

jdrueckert commented 2 years ago

@pree-T Generally yes. It does need a decent bit of investigation first, though. But sure, go ahead, give it a try and ideally first drop your findings about broken or ignored tests in here before starting to try to fix them :wink:

soloturn commented 1 year ago

when building with java-11 it ends with:

❯ gradle clean build
...
> Task :subsystems:TypeHandlerLibrary:spotbugsTest
M B PI: The used identifier ?>?2/2??? as field name in class org.terasology.persistence.typeHandling.InMemorySerializerTest$TypeGetter.STRING in source file ?>?3/2??? shadows the publicly available identifier from the Java Standard Library.  In InMemorySerializerTest.java
M C EC: Using .equals to compare two byte[]'s, (equivalent to ==) in org.terasology.persistence.typeHandling.coreTypes.factories.BytesTypeHandlerTest.byteArraySerializeDeserialize()  At BytesTypeHandlerTest.java:[line 42]
M C EC: Using .equals to compare two byte[]'s, (equivalent to ==) in org.terasology.persistence.typeHandling.coreTypes.factories.BytesTypeHandlerTest.byteArraySerializeDeserialize()  At BytesTypeHandlerTest.java:[line 45]
M C EC: Using .equals to compare two byte[]'s, (equivalent to ==) in org.terasology.persistence.typeHandling.InMemorySerializerTest.serializeBytes()  At InMemorySerializerTest.java:[line 309]
M C EC: Using .equals to compare two byte[]'s, (equivalent to ==) in org.terasology.persistence.typeHandling.InMemorySerializerTest.serializeByteBuffer()  At InMemorySerializerTest.java:[line 338]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.serializeString()  At InMemorySerializerTest.java:[line 82]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.serializeStrings()  At InMemorySerializerTest.java:[line 113]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.template(PersistedData)  At InMemorySerializerTest.java:[line 136]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.serializeBoolean()  At InMemorySerializerTest.java:[line 255]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.serializeBytes()  At InMemorySerializerTest.java:[line 319]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.serializeByteBuffer()  At InMemorySerializerTest.java:[line 348]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.serializeNull()  At InMemorySerializerTest.java:[line 392]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.checkIsArray(PersistedData)  At InMemorySerializerTest.java:[line 434]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.checkIsNumber(PersistedData)  At InMemorySerializerTest.java:[line 458]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.checkValueArray(PersistedData, PersistedData, Set)  At InMemorySerializerTest.java:[line 486]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.lambda$checkValueArray$8(Executable)  At InMemorySerializerTest.java:[line 512]
H C RV: org.junit.jupiter.api.Assertions.assertThrows(Class, Executable) not thrown in org.terasology.persistence.typeHandling.InMemorySerializerTest.lambda$checkValueArray$4(Executable)  At InMemorySerializerTest.java:[line 497]
M P UuF: Unused field: org.terasology.persistence.typeHandling.coreTypes.factories.ObjectFieldMapTypeHandlerFactoryTest$SomeClass.t  In ObjectFieldMapTypeHandlerFactoryTest.java
M P SIC: Should org.terasology.persistence.typeHandling.FutureTypeHandlerTest$ResultCaptor be a _static_ inner class?  At FutureTypeHandlerTest.java:[lines 41-50]
M P UuF: Unused field: org.terasology.persistence.typeHandling.coreTypes.factories.ObjectFieldMapTypeHandlerFactoryTest$SomeClass.list  In ObjectFieldMapTypeHandlerFactoryTest.java
SpotBugs ended with exit code 1

> Task :subsystems:TypeHandlerLibrary:spotbugsMain
SpotBugs ended with exit code 1

i'll give to fix these ones a try.