Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider entirely removing the configuration #26

Open
BenWoodworth opened this issue Mar 6, 2024 · 1 comment
Open

Consider entirely removing the configuration #26

BenWoodworth opened this issue Mar 6, 2024 · 1 comment

Comments

@BenWoodworth
Copy link
Owner

BenWoodworth commented Mar 6, 2024

(WIP: still expanding on my thoughts in this issue)

made sense with contextual options

  • in general
  • decorator
    • Awkward now that parameterize is inline, requiring the decorator to suspend
    • Easy to match functionality by wrapping parameterize with a specialized function
      • Which is more flexible & powerful
  • onFailure
    • Makes assumptions about failures that won't fit all projects
      • e.g. with recordFailure, there's no consideration for skip and abort exceptions and how they should be counted in the ParameterizeFailedError. This may be best left to parameterize users to decide

Removing the configuration would also mean the block can be marked as callsInPlace. It can't currently because it's wrapped in a try-catch, but since the default parameterize effectively doesn't catch at all, meaning the try-catch could be removed.

@BenWoodworth
Copy link
Owner Author

BenWoodworth commented Mar 6, 2024

An important consideration is how the recommendation for kotlin.test would have to be re-written.
In v0.3.0, this is the suggested way to write it:

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()
}

Without configuration options, and with an API like in #25 to access similar functionality without the configuration, re-writing might look like this if exactly matching the old behavior (with // potential enhancements):

protected inline fun parameterizeTestNew(
    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
) {
    var passed = 0L
    var failed = 0L
//  var skipped = 0L

    var completedEarly = false
    val failures = mutableListOf<ParameterizeFailure>()

    run { // To break out of the `parameterize` loop
        parameterize {
            if (!parameterizeContext.isFirstIteration) {
                beforeTest()
            }

            try {
                block()
                passed++
            } catch (parameterizeException: ParameterizeException) { // Should not catch its own `ParameterizeException`
                throw parameterizeException // PROBLEM: Should be caught as `testFailure` if from another `parameterize`
//          } catch (_: TestSkippedException) {
//              skipped++
//          } catch (_: IncompleteExecutionException) { // Not a test failure
//              // continue
            } catch (testFailure: Throwable) {
                failed++
                if (failures.size < recordFailures) {
                    failures += ParameterizeFailure(testFailure)
                }
                if (failed >= maxFailures && !parameterizeContext.isLastIteration) {
                    completedEarly = true
                    return@run
                }
            } finally {
                if (!parameterizeContext.isLastIteration) {
                    afterTest()
                }
            }
        }
    }
    
    if (failed > 0) {
        val message = buildString {
            val total = passed + failed //+ skipped
            append("Failed $failed/$total")
            
            if (completedEarly) append('+')
            append(" cases")
//          if (skipped > 0) append("($skipped skipped)")
        }
        
        throw ParameterizeFailedError(message, failures)
    }
}

This clearly isn't ideal because of how long it it, so it would need to be greatly reduced one way or another. for example:

  • cutting back on the example, like removing the parameters and their logic
  • providing helper functions or a dsl for creating the parameterized test function in a sister library (e.g. parameterize-test)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant