search

Terasology / NameGenerator

This module adds name generators.
2 stars 9 forks source link

chore: migrate test to junit5 #28

Closed pollend closed 3 years ago

pollend commented 3 years ago

PR: https://github.com/MovingBlocks/Terasology/pull/4563

keturn commented 3 years ago

Another user of HeadlessEnvironment from engine-test, similar to Pathfinding.

Here, though, I question the necessity of building an entire engine environment to test some strings https://github.com/Terasology/NameGenerator/blob/afc8b47ed2a8597b8d9ff8e4668fb4b490ff3136/src/test/java/org/terasology/namegenerator/TownNameProviderTest.java#L44-L45

are there any test cases in this module that couldn't be covered by more focused unit tests?

pollend commented 3 years ago

Another user of HeadlessEnvironment from engine-test, similar to Pathfinding.

Here, though, I question the necessity of building an entire engine environment to test some strings

https://github.com/Terasology/NameGenerator/blob/afc8b47ed2a8597b8d9ff8e4668fb4b490ff3136/src/test/java/org/terasology/namegenerator/TownNameProviderTest.java#L44-L45

are there any test cases in this module that couldn't be covered by more focused unit tests?

It relies on some logic from the asset system so its using the test environment but its breaking with reflection and I'm not sure why?

keturn commented 3 years ago 
java.lang.RuntimeException: org.reflections.ReflectionsException: Scanner TypeAnnotationsScanner was not configured

is something that has come up with using MTE on the Cities module, which is probably in use here with this thing that generates modules for cities.

https://github.com/MovingBlocks/Terasology/pull/4543 probably helps (by ignoring that exception)

It'd be neat if upgrading reflections helped (if they decided it was a bug on that side and it was fixed there), but I haven't checked. https://github.com/MovingBlocks/Terasology/pull/4564