Replace contextual `parameterize` configuration with a better alternative
BenWoodworth opened this issue · 2 comments
The current approach to sharing a configuration via context in v0.2 can be awkward to use, and prone to error.
It was originally introduced as a way to nicely share a configuration contextually/implicitly to all parameterize
calls in scope. Especially with the idea of scope properties being proposed for Kotlin, allowing all calls in the same class or file (or maybe project-wide, if something like C#'s implicit global imports were ever added, though that was just me being hopeful).
Leaning into future/undecided language features has been something I've been iffy about, especially since a new context proposal was introduced 4 days after this issue was created, and especially especially since the scope properties are only a "potential future enhancement" to the proposal.
Given all that though, even if context receivers and scope properties were already in Kotlin, I've started to lean away from a contextual parameterize
overload being a good idea after having used it for a bit now. The original proposal advised against it, and it's starting to become clear why. Because the context is implicit and the signatures are the same, it's very easy to miss if the context isn't in scope resulting in the non-contextual overload being used. Whether that's because the context wasn't setup correctly, or the contextual parameterize
call was copied somewhere else expecting it to work the same, it's an easy for hard-to-debug problems to occur. It also doesn't have the most ergonomic tooling with IntelliJ, since the IntelliSense completion will insert a fully-qualified non-contextual com.benwoodworth.parameterize.parameterize {}
call when a ParameterizeContext
is in scope (which won't be a problem when once the signature's changed from using an extension receiver to a context receiver, but for now, it's an issue).
Encouraging devs to wrap parameterize themselves
abstract class TestingContext {
private val parameterizeTestConfiguration = ParameterizeConfiguration {
decorator = { iteration ->
if (!isFirstIteration) beforeTest()
iteration()
if (!isLastIteration) afterTest()
}
onFailure = {
recordFailure = failureCount <= 10
}
}
protected inline fun parameterizeTest(block: ParameterizeScope.() -> Unit): Unit =
parameterize(parameterizeTestConfiguration, block)
// ...
}
- Pros:
- More control to the library user, since they may not want calling code to be able to override the decorator, for example.
- Cons:
- Would add boilerplate if library users do want to have all the same parameters, and having them be overridable
- ^plus, if new parameters are added, the wrapping function will need to be manually updated
Pre-configured invoke
-able class
Instead, having a pre-configured parameterize val
that can be invoked would be more flexible
- can be named appropriately
- vs. composing, since options don't need to be overridden
Example:
abstract class TestingContext {
protected override val parameterizeTest = Parameterize {
// Insert before/after calls around each parameterized
// test case, except when already invoked by kotlin.test
decorator = { iteration ->
if (!isFirstIteration) beforeTest()
iteration()
if (!isLastIteration) afterTest()
}
// ...other shared configuration
}
open fun beforeTest() {}
open fun afterTest() {}
// The annotations would be lost when overriding beforeTest/afterTest,
// so hook in here instead of relying on the subclass to apply them.
@BeforeTest
fun beforeTestHook(): Unit = beforeTest()
@AfterTest
fun afterTestHook(): Unit = afterTest()
}
class KotlinTestSpec : TestingContext() {
override fun beforeTest() {
// ...
}
override fun afterTest() {
// ...
}
@Test
fun a_string_should_contain_its_own_substring() = parameterizeTest {
val substring = "substring"
val prefix by parameterOf("prefix-", "")
val suffix by parameterOf("-suffix", "")
val string = "$prefix$substring$suffix"
assertTrue(string.contains(substring), "\"$string\".contains(\"$substring\")")
}
}
I'm leaning towards the wrapping approach, since can't think of a use case for pre-configuring and needing all options to be potentially overridable. If a good use case ever does arise, I can add in a solution that addresses that need later, instead of defensively providing an API for that now. It also wouldn't be hard to almost match fully overridable configuration options without much boilerplate. For example:
internal abstract class TestingContext {
private val parameterizeTestConfiguration = ParameterizeConfiguration {
decorator = { iteration ->
if (!isFirstIteration) beforeTest()
iteration()
if (!isLastIteration) afterTest()
}
onFailure = {
recordFailure = failureCount <= 10
}
}
protected inline fun parameterizeTest(
noinline configure: (ParameterizeConfiguration.Builder.() -> Unit)? = null,
block: ParameterizeScope.() -> Unit
) {
// Avoid building new configuration if no options are changed
val configuration = if (configure != null) {
ParameterizeConfiguration(parameterizeTestConfiguration, configure)
} else {
parameterizeTestConfiguration
}
parameterize(configuration, block)
}
// ...
}
And then with only slightly less appealing syntax with {}
wrapping the options:
parameterizeTest({
// Override parameterizeTest's default onFailure
onFailure = { recordFailure = true }
}) {
// ...
}
Composing a function with parameterize
is proving to actually be a really powerful pattern. Maybe even more powerful than using a configuration because it can manage its own state (e.g. the skipped
count, which parameterize
doesn't track).
Take the example I ended up endorsing for kotlin.test, for instance:
protected inline fun parameterizeTest(
recordFailures: Long = someDefault, // Example of how `parameterize` could get wrapped,
maxFailures: Long = Long.MAX_VALUE, // exposing options according to the testing needs.
block: ParameterizeScope.() -> Unit
): Unit = parameterize(
// Inserts before & after calls around each test case,
// except where already invoked by the test framework.
decorator = { testCase ->
if (!isFirstIteration) beforeTest()
testCase()
if (!isLastIteration) afterTest()
},
onFailure = { failure ->
recordFailure = failureCount <= recordFailures
breakEarly = failureCount >= maxFailures
}
) {
block()
}
Conceivably, it could be re-written without a configuration at all, plus have its own state which isn't possible with the ParameterizeConfiguration
:
protected inline fun parameterizeTest(
recordFailures: Long = someDefault,
maxFailures: Long = Long.MAX_VALUE,
block: ParameterizeScope.() -> Unit
){
var passed = 0
var failed = 0
var skipped = 0
val recordedFailures = mutableListOf<Throwable>()
var isFirstTestCase = true
run {
parameterize {
try {
if (!isFirstTestCase) {
afterTest()
beforeTest()
}
isFirstTestCase = false
block()
passed++
} catch (failure: Throwable) {
when (failure) {
is ParameterizeFailedException -> throw failure
is TestSkippedException -> skipped++
else -> {
failed++
if (failed <= recordFailures) recordedFailures += failure
if (failed >= maxFailures) return@run
}
}
}
}
}
if (failed > 0) {
throw ParameterizeFailedError()
}
}
Of course there are some things missing, like the current parameters/values aren't exposed through the ParameterizeScope's API, and the configuration does help to greatly reduce the boilerplate, but this does show just how powerful this approach is.