Use Kotlintest for unit-testing styx routing objects #394
Conversation
mikkokar
requested review from
kvosper and
dvlato
and removed request for
kvosper
...rc/test/java/com/hotels/styx/metrics/reporting/graphite/NonSanitizingGraphiteSenderTest.java
Outdated
Show resolved
Hide resolved
components/proxy/src/test/kotlin/com/hotels/styx/ServerConfigSchemaTest.kt
Show resolved
Hide resolved
| assert(backendServices.toList().size == 2) | ||
| assert(backendServices.first() == backendServiceOne) | ||
|
|
||
| StyxFutures.await(registry.stop()) |
There was a problem hiding this comment.
do we need to wait for the stop? I suppose that means we cannot run the tests in parallel, it would be nice to improve that...
There was a problem hiding this comment.
Okay. I'll keep this in mind. This problem is inherited from the original Scala test. So it isn't introduced by this PR.
| } | ||
|
|
||
| fun createOnMissing(path: String): String { | ||
| val absolutePath = Paths.get(javaClass.getResource("/").toURI()).resolve(path).toFile() |
There was a problem hiding this comment.
Why are you using both java.nio and java.io here? is it because writeValue() above require a File?
There was a problem hiding this comment.
This is also inherited from the original tests.
| import java.util.concurrent.CompletableFuture | ||
| import java.util.concurrent.CompletableFuture.completedFuture | ||
|
|
||
| class BackendServiceProxyTest: StringSpec ({ |
There was a problem hiding this comment.
There's an space before the parenthesis, which is not consistent with the other tests.
| (routingObjectDef as RouteHandlerDefinition).name() shouldBe("main-router") | ||
| routingObjectDef.type() shouldBe("ConditionRouter") | ||
|
|
||
| routingObjectDef.config().shouldBeInstanceOf<JsonNode>() |
There was a problem hiding this comment.
Since we saw that in case of an assertion error this only prints the objects toString() and not the classname, we might want to somehow include a message with the actual class name .
components/proxy/src/test/kotlin/com/hotels/styx/ServerConfigSchemaTest.kt
Outdated
Show resolved
Hide resolved
...src/test/kotlin/com/hotels/styx/proxy/backends/file/FileBackedBackendServicesRegistryTest.kt
Outdated
Show resolved
Hide resolved
Disable NonSanitizingGraphiteSenderTest (uses java internal classes).
- ConfigVersionResolverSpec - RouteHandlerFactorySpec - RoutingConfigParserSpec
- RoutingConfigParserSpec - ConditionRouterConfigSpec - RewriteInterceptorSpec
- ServerConfigSchemaSpec - StaticResponseHandlerSpec - ProxyToBackendSpec - HttpInterceptorPipelineSpec - BackendServiceProxySpec - FileBackkedBackendServicesRegistrySpec
mikkokar
force-pushed
the
refactor-styx-routing
branch
from
7a274a0 to
7f418f3
Compare
Summary
Rationale:
Routing object tests rely heavily on yaml configuration that require multi-line strings. Both Kotlin & Scala support multi-line strings and are handy for this domain. But Kotlin collections and SAM conversions interoperate better with existing Java code. Perhaps it is just my Scala knowledge (or lack of) but Kotlin seems more effortless for this purpose.