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

Added checks in CoyoteRuntime.RunTestAsync() to check testMethod if it is either a Action<ICoyoteRuntime> or Func<ICoyoteRuntime, Task> delegate type before calling the runtime extension RunTest() method #509

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kzdev-net
Copy link

The runTest that acts as the top-level execution for the ControlledThread that is created in CoyoteRuntime.RunTestAsync first calls
if (this.Extension.RunTest(testMethod, out Task extensionTask))
before checking if testMethod is an Action or a Func.
When this.Extension is an ActorExecutionContext, then the RunTest method is implemented like so

        bool IRuntimeExtension.RunTest(Delegate test, out Task task)
        {
            if (test is Action<IActorRuntime> actionWithRuntime)
            {
                actionWithRuntime(this);
                task = Task.CompletedTask;
                return true;
            }
            else if (test is Func<IActorRuntime, Task> functionWithRuntime)
            {
                task = functionWithRuntime(this);
                return true;
            }

            task = Task.CompletedTask;
            return false;
        }

However, the Action<T> and Func<T,Task> generic parameter 'T' is contravarient. So, if the testMethod is either an Action<ICoyoteRuntime> or a Func<ICoyoteRuntime, Task>, one of the condition tests will pass, and the method will be executed. Which is fine, but it masks/hides a possible condition where if the runtime extension is not an ActorExecutionContext instance and instead returns false, then the conditional checks in the runTest delegate for the ControlledThread will throw an exception stating that the test delegate type is unsupported even though the TestEngine.Create overloads allow and support such a test method.


        /// <summary>
        /// Creates a new systematic testing engine.
        /// </summary>
        public static TestingEngine Create(Configuration configuration, Action<ICoyoteRuntime> test) =>
            new TestingEngine(configuration, test, new LogWriter(configuration, true));

        /// <summary>
        /// Creates a new systematic testing engine.
        /// </summary>
        public static TestingEngine Create(Configuration configuration, Func<ICoyoteRuntime, Task> test) =>
            new TestingEngine(configuration, test, new LogWriter(configuration, true));

There needs to be type checks in the testMethod prior to calling this.Extension.RunTest().


      if (testMethod is Action<ICoyoteRuntime> actionWithRuntime)
      {
          actionWithRuntime(this);
      }
      else if (testMethod is Func<ICoyoteRuntime, Task> functionWithRuntime)
      {
          task = functionWithRuntime(this);
      }
      else if (this.Extension.RunTest(testMethod, out Task extensionTask))
      {
        ...
      }

…t is either a Action<ICoyoteRuntime> or Func<ICoyoteRuntime, Task> delegate type before calling the runtime extension RunTest() method because for an extension of type ActorExecutionContext, the RunTest method will gladly call one of those delegate types even though it is designed to only handle Action<IActorRuntime> or Func<IActorRuntime, Task>delegate types, but Action<T> and Func<T, Task> generics have a contravarient generic parameter, so ActorExecutionContext will indicate that the delegate was called fine, but other extension types (e.g. NullRuntimeExtension) will not call the methods, so the provided test method might only run if the ActorExecutionContext is the runtime extension, but the test method is not actually the expected type of delegate.
@kzdev-net
Copy link
Author

@kzdev-net please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

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

Successfully merging this pull request may close these issues.

1 participant