JetBrains / lincheck

Framework for testing concurrent data structures
Mozilla Public License 2.0
545 stars 31 forks source link

Strategy run API refactoring #262

Closed eupp closed 3 weeks ago

eupp commented 5 months ago

This PR proposes a simple refactoring of the Strategy class API.

Motivation

Currently, the Strategy API provides the following method to run the scenario:

abstract fun run(): LincheckFailure?

This method should run the scenario (some) number of times and returns the failure, in case if any invocation fails.

This API leads to code duplication in the current code base.

This PR proposes to change the API, so that the Strategy instead provides the method to run single invocation:

abstract fun runInvocation(): InvocationResult

This change allows to implement the previously duplicated functionality in a single place, as a bunch of Strategy interface extension methods.

Note that in addition, the new API adds two (optional) methods to implement:

    /**
     * Sets the internal state of strategy to run the next invocation.
     *
     * @return true if there is next invocation to run, false if all invocations have been studied.
     */
    open fun nextInvocation(): Boolean = true

    /**
     * Initializes the invocation.
     * Should be called before each call to [runInvocation].
     */
    open fun initializeInvocation() {}

Thus, the pattern to run the strategy becomes as follows:

// run single invocation (if available)
with(strategy) {
     if (nextInvocation()) {
        initializeInvocation()
        runInvocation()
     }
}

For deterministic strategies (like model checking strategy), consecutive calls of runInvocation withouth nextInvocation calls, should lead to the same results:

with(strategy) {
     if (nextInvocation()) {
        // first invocation run
        initializeInvocation()
        val r1 = runInvocation()
        // second invocation run
        initializeInvocation()
        val r2 = runInvocation()
        // two invocations should return the same result
        check(r1 == r2)
     }
}

This change also simplifies implementation of the new features.

ndkoval commented 4 months ago

@eupp could you please rebase the PR onto develop?

ndkoval commented 1 month ago

@eupp could you please rebase the PR?

eupp commented 1 month ago

@eupp could you please rebase the PR?

Rebased.

ndkoval commented 3 weeks ago

@eupp please address the remaining minor comments and merge the PR 🎉