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

Support Scenario-Level Parallelization for MsTest #119

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

obligaron
Copy link
Contributor

Behaviour changes

  • Support Scenario-Level Parallelization for MsTest (ExecutionScope.MethodLevel)
  • MsTest: Each Scenario has it's own/new ITestRunner, ITestExecutionEngine and IContextManager instance (before one instance was reused)
  • ITestRunner, ITestExecutionEngine and IContextManager have new methods

Implementation notes

  • BeforeFeature/AfterFeature is only called once. This is done with a static ITestRunner (same behaviour as before). The ITestRunner needs to be created, because it can be injected as an (optional) parameter into the BeforeFeature/AfterFeature methods.
  • To ensure that no state is shared between Scenarios a new ITestRunner/ITestExecutionEngine/IContextManager is created for each Scenario.
  • The new objects are registered in the BoDi-IObjectContainer at scenario-level. So hopefully they should not share state and the DI-Logic will still work for them. They override the feature-level objects.
  • Pass IContextManager to IStepArgumentTypeConverter to make it independent from DI (needed now there are multiple instances of IContextManager in DI)

Open questions

  • Currently the per Scenario objects are created with new instead of the BoDi-Logic. Can a new instance be registered in a child-IObjectContainer when a instance already exists in the parent-IObjectContainer?
  • Should the per-scenario ITestRunner be registered in the ITestRunnerManager?
  • How should TestWorkerId be handled in the per-scenario ITestRunner? (Currently null)
  • Could this be used as a basis for NUnit support?
  • I wasn't sure if this should be classified as bug fix or new feature.
  • I wasn't sure if the behaviour change (new instance of ITestRunner, ITestExecutionEngine and IContextManager per scenario) should be classified as a breaking change. They should work the same as before (they have the same information etc.) but technically they are not the same instance. So if someone puts them in a static field, they might see some differences in the behaviour.
  • I tried to push a branch to the main repro, but got the following error:

    Pushing parallel
    Remote: Permission to reqnroll/Reqnroll.git denied to obligaron.
    Error encountered while pushing to the remote repository: Git failed with a fatal error.
    Git failed with a fatal error.
    unable to access 'https://github.com/reqnroll/Reqnroll/': The requested URL returned error: 403
    I checked "Allow edits by maintainers", so collaboration should still be possible.

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to not work as expected).
  • Performance improvement
  • Refactoring (so no functional change)
  • Other (docs, build config, etc)

Checklist:

  • I've added tests for my code. (most of the time mandatory)
  • I have added an entry to the changelog. (mandatory)
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

I'm glad that Reqnroll exists and that there is a future for BDD/Cucumber in the .NET ecosystem. This is my first major change for the project. I hope you like it and I'm looking forward to your feedback. 🙂

@obligaron obligaron changed the title Support Scenario-Level Parallelization for MSTest Support Scenario-Level Parallelization for MsTest May 5, 2024
@gasparnagy
Copy link
Contributor

@obligaron Thx. I will check the permission issue.

I need a bit more time to make a proper review of this, but have three questions/notes that we can start thinking on already:

  1. When you did the tests. Have you seen them failing? Could you share the stack tract of the failures or temporary disable the implementation (if possible) to let us see the error on CI?
  2. The static feature test runner is problematic. We will need to find an alternative way, but I need a bit more time to give a suggestion.
  3. The "Specs" project will be repurposed to contain only specification related scenarios only, not technical tests. Currently it is only running with MsTest. The new place for such tests is the "SystemTests" project, but not all tests have been migrated yet to the new project. (Particularly the parallel execution tests not yet.) So these tests would need to be ported to SystemTests (e.g. to https://github.com/reqnroll/Reqnroll/blob/main/Tests/Reqnroll.SystemTests/Generation/MsTestGenerationTest.cs, but even better if you create a separate class for these.) This is a bit of extra work, but the SystemTests uses the same infrastructure, so the porting is not that horrible.

@obligaron
Copy link
Contributor Author

Take your time. We are in no hurry. 🙂

1. When you did the tests. Have you seen them failing? Could you share the stack tract of the failures or temporary disable the implementation (if possible) to let us see the error on CI?

Yes the tests were failing before. When I run my newly added test with main branch. They fail with both settings (the new test verifies that the test runs parallel).

Running with `ExecutionScope.MethodLevel` in main branch
    Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
MSTest Executor: Test Parallelization enabled for temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\bin\Debug\net8.0\TestProj_fe593232.dll (Workers: 4, Scope: MethodLevel).
  Failed SimpleScenarioOutline4 [58 ms]
  Error Message:
   Test method TestProj_fe593232.Feature1Feature.SimpleScenarioOutline4 threw exception: 
System.NullReferenceException: Object reference not set to an instance of an object.
  Stack Trace:
      at Reqnroll.Infrastructure.ContextManager.StackedInternalContextManager`1.RemoveTop()
   at Reqnroll.Infrastructure.ContextManager.CleanupStepContext()
   at Reqnroll.Infrastructure.TestExecutionEngine.StepAsync(StepDefinitionKeyword stepDefinitionKeyword, String keyword, String text, String multilineTextArg, Table tableArg)
   at Reqnroll.TestRunner.WhenAsync(String text, String multilineTextArg, Table tableArg, String keyword)
   at TestProj_fe593232.Feature1Feature.SimpleScenarioOutline4() in temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\FeatureFile766741313320410097157c2e045098bb.feature:line 24
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.ThreadOperations.ExecuteWithAbortSafety(Action action)

  Standard Output Messages:
 

TestContext Messages:
-> warning: The previous ScenarioContext was not disposed.-> warning: The previous ScenarioContext was not disposed.-> warning: The previous ScenarioContext was not disposed.
 
 


  Passed SimpleScenarioOutline3 [1 s]
  Passed SimpleScenarioOutline5 [1 s]
  Failed SimpleScenarioOutline2 [1 s]
  Standard Output Messages:
 -> Loading plugin temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\bin\Debug\net8.0\Reqnroll.MSTest.ReqnrollPlugin.dll
 -> Loading plugin temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\bin\Debug\net8.0\TestProj_fe593232.dll
 -> Using reqnroll.json


  Failed SimpleScenarioOutline2 (1,,)
  Error Message:
   Exception thrown while executing test. If using extension of TestMethodAttribute then please contact vendor. Error message: Collection was modified; enumeration operation may not execute.

  Failed SimpleScenarioOutline2 (2,,) [1 ms]
  Error Message:
   Test method TestProj_fe593232.Feature1Feature.SimpleScenarioOutline2 threw exception: 
System.NullReferenceException: Object reference not set to an instance of an object.
  Stack Trace:
      at lambda_method36(Closure, IContextManager, String)
   at InvokeStub_Action`2.Invoke(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
--- End of stack trace from previous location ---
   at Reqnroll.Bindings.BindingInvoker.InvokeBindingAsync(IBinding binding, IContextManager contextManager, Object[] arguments, ITestTracer testTracer, DurationHolder durationHolder)
   at Reqnroll.Infrastructure.TestExecutionEngine.ExecuteStepMatchAsync(BindingMatch match, Object[] arguments, DurationHolder durationHolder)
   at Reqnroll.Infrastructure.TestExecutionEngine.ExecuteStepAsync(IContextManager contextManager, StepInstance stepInstance)
   at Reqnroll.Infrastructure.TestExecutionEngine.OnAfterLastStepAsync()
   at Reqnroll.TestRunner.CollectScenarioErrorsAsync()
   at TestProj_fe593232.Feature1Feature.ScenarioCleanupAsync()
   at TestProj_fe593232.Feature1Feature.SimpleScenarioOutline2(String count, String notUsed6248, String[] exampleTags) in temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\FeatureFile766741313320410097157c2e045098bb.feature:line 12
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.ThreadOperations.ExecuteWithAbortSafety(Action action)

  Standard Output Messages:
 

TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 2'


  Passed SimpleScenarioOutline2 (3,,) [1 s]
  Failed SimpleScenarioOutline1 [1 s]
  Failed SimpleScenarioOutline1 (1,,) [57 ms]
  Error Message:
   Test method TestProj_fe593232.Feature1Feature.SimpleScenarioOutline1 threw exception: 
System.NullReferenceException: Object reference not set to an instance of an object.
  Stack Trace:
      at Reqnroll.Infrastructure.ContextManager.StackedInternalContextManager`1.RemoveTop()
   at Reqnroll.Infrastructure.ContextManager.CleanupStepContext()
   at Reqnroll.Infrastructure.TestExecutionEngine.StepAsync(StepDefinitionKeyword stepDefinitionKeyword, String keyword, String text, String multilineTextArg, Table tableArg)
   at Reqnroll.TestRunner.WhenAsync(String text, String multilineTextArg, Table tableArg, String keyword)
   at TestProj_fe593232.Feature1Feature.SimpleScenarioOutline1(String count, String notUsed6248, String[] exampleTags) in temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\FeatureFile766741313320410097157c2e045098bb.feature:line 3
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.ThreadOperations.ExecuteWithAbortSafety(Action action)

Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
-> warning: The previous ScenarioStepContext was already disposed.
-> warning: The previous ScenarioContext was already disposed.
  Failed SimpleScenarioOutline1 (2,,) [< 1 ms]
  Error Message:
   Test method TestProj_fe593232.Feature1Feature.SimpleScenarioOutline1 threw exception: 
System.NullReferenceException: Object reference not set to an instance of an object.
  Stack Trace:
      at Reqnroll.Infrastructure.ContextManager.StackedInternalContextManager`1.RemoveTop()
   at Reqnroll.Infrastructure.ContextManager.StackedInternalContextManager`1.Reset()
   at Reqnroll.Infrastructure.ContextManager.ResetCurrentStepStack()
   at Reqnroll.Infrastructure.ContextManager.InitializeScenarioContext(ScenarioInfo scenarioInfo)
   at Reqnroll.Infrastructure.TestExecutionEngine.OnScenarioInitialize(ScenarioInfo scenarioInfo)
   at Reqnroll.TestRunner.OnScenarioInitialize(ScenarioInfo scenarioInfo)
   at TestProj_fe593232.Feature1Feature.ScenarioInitialize(ScenarioInfo scenarioInfo)
   at TestProj_fe593232.Feature1Feature.SimpleScenarioOutline1(String count, String notUsed6248, String[] exampleTags) in temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\FeatureFile766741313320410097157c2e045098bb.feature:line 2
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.ThreadOperations.ExecuteWithAbortSafety(Action action)

  Standard Output Messages:
 -> warning: The previous ScenarioContext was not disposed.
 When I do something in Scenario 'Simple Scenario Outline 2'
 

TestContext Messages:
-> warning: The previous ScenarioContext was not disposed.


  Failed SimpleScenarioOutline1 (3,,) [1 s]
  Error Message:
   Test method TestProj_fe593232.Feature1Feature.SimpleScenarioOutline1 threw exception: 
System.NullReferenceException: Object reference not set to an instance of an object.
TestCleanup method TestProj_fe593232.Feature1Feature.TestTearDownAsync threw exception. System.NullReferenceException: System.NullReferenceException: Object reference not set to an instance of an object..
  Stack Trace:
      at Reqnroll.Infrastructure.TestExecutionEngine.GetHookContainer(HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.FireRuntimePluginTestExecutionLifecycleEvents(HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.FireEventsAsync(HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.FireScenarioEventsAsync(HookType bindingEvent)
   at Reqnroll.Infrastructure.TestExecutionEngine.OnStepEndAsync()
   at Reqnroll.Infrastructure.TestExecutionEngine.ExecuteStepAsync(IContextManager contextManager, StepInstance stepInstance)
   at Reqnroll.Infrastructure.TestExecutionEngine.StepAsync(StepDefinitionKeyword stepDefinitionKeyword, String keyword, String text, String multilineTextArg, Table tableArg)
   at Reqnroll.TestRunner.WhenAsync(String text, String multilineTextArg, Table tableArg, String keyword)
   at TestProj_fe593232.Feature1Feature.SimpleScenarioOutline1(String count, String notUsed6248, String[] exampleTags) in temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\FeatureFile766741313320410097157c2e045098bb.feature:line 3
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.ThreadOperations.ExecuteWithAbortSafety(Action action)

TestCleanup Stack Trace
   at Reqnroll.Infrastructure.TestExecutionEngine.OnScenarioEndAsync()
   at Reqnroll.TestRunner.OnScenarioEndAsync()
   at TestProj_fe593232.Feature1Feature.TestTearDownAsync()

  Standard Output Messages:
 When I do something in Scenario 'Simple Scenario Outline 1'
 -> error: Object reference not set to an instance of an object. (0,0s)
 Start index: 2, Worker: 14
 

TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 1'
 -> error: Object reference not set to an instance of an object. (0,0s)
 Start index: 2, Worker: 14
 -> warning: The previous ScenarioStepContext was already disposed.
 -> warning: The previous ScenarioContext was already disposed.


  Failed SimpleScenarioOutline6 [1 s]
  Error Message:
   Test method TestProj_fe593232.Feature1Feature.SimpleScenarioOutline6 threw exception: 
System.NullReferenceException: Object reference not set to an instance of an object.
TestCleanup method TestProj_fe593232.Feature1Feature.TestTearDownAsync threw exception. System.NullReferenceException: System.NullReferenceException: Object reference not set to an instance of an object..
  Stack Trace:
      at Reqnroll.Infrastructure.TestExecutionEngine.GetHookContainer(HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.FireRuntimePluginTestExecutionLifecycleEvents(HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.FireEventsAsync(HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.FireScenarioEventsAsync(HookType bindingEvent)
   at Reqnroll.Infrastructure.TestExecutionEngine.OnStepEndAsync()
   at Reqnroll.Infrastructure.TestExecutionEngine.ExecuteStepAsync(IContextManager contextManager, StepInstance stepInstance)
   at Reqnroll.Infrastructure.TestExecutionEngine.StepAsync(StepDefinitionKeyword stepDefinitionKeyword, String keyword, String text, String multilineTextArg, Table tableArg)
   at Reqnroll.TestRunner.WhenAsync(String text, String multilineTextArg, Table tableArg, String keyword)
   at TestProj_fe593232.Feature1Feature.SimpleScenarioOutline6() in temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\FeatureFile766741313320410097157c2e045098bb.feature:line 30
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.ThreadOperations.ExecuteWithAbortSafety(Action action)

TestCleanup Stack Trace
   at Reqnroll.Infrastructure.TestExecutionEngine.OnScenarioEndAsync()
   at Reqnroll.TestRunner.OnScenarioEndAsync()
   at TestProj_fe593232.Feature1Feature.TestTearDownAsync()

  Standard Output Messages:
 When I do something in Scenario 'Simple Scenario Outline 6'
 Start index: 5, Worker: 14
 Was parallel
 Was parallel
 Was parallel
 -> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
 -> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
 -> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
 -> warning: The previous ScenarioStepContext was already disposed.
 

TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 6'
 Start index: 5, Worker: 14
 Was parallel
 Was parallel
 Was parallel
 -> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
 
 -> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
 -> warning: The previous ScenarioStepContext was already disposed.


Results File: temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\TestResults\obligaron_DESKTOP-CEF20VJ_2024-05-07_21_34_50.trx

Total tests: 10
     Passed: 3
     Failed: 7
 Total time: 2,6486 Seconds
     1>Project "temp\RR\R2c3c025d\S859d1238\S859d1238.sln" (1) is building "temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\TestProj_fe593232.csproj" (2) on node 1 (VSTest target(s)).
     2>_VSTestConsole:
         MSB4181: The "VSTestTask" task returned false but did not log an error.
     2>Done Building Project "temp\RR\R2c3c025d\S859d1238\TestProj_fe593232\TestProj_fe593232.csproj" (VSTest target(s)) -- FAILED.
     1>Done Building Project "temp\RR\R2c3c025d\S859d1238\S859d1238.sln" (VSTest target(s)) -- FAILED.

Build FAILED.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:03.07

StdError:
Test Run Failed.
Running with `ExecutionScope.ClassLevel` in main branch
    Starting test execution, please wait...
A total of 1 test files matched the specified pattern.
MSTest Executor: Test Parallelization enabled for temp\RR\R69c59efd\S50c277a4\TestProj_0920d173\bin\Debug\net8.0\TestProj_0920d173.dll (Workers: 4, Scope: ClassLevel).
  Passed SimpleScenarioOutline1 [3 s]
  Passed SimpleScenarioOutline1 (1,,) [1 s]
  Passed SimpleScenarioOutline1 (2,,) [1 s]
  Passed SimpleScenarioOutline1 (3,,) [1 s]
  Passed SimpleScenarioOutline2 [3 s]
  Passed SimpleScenarioOutline2 (1,,) [1 s]
  Passed SimpleScenarioOutline2 (2,,) [1 s]
  Passed SimpleScenarioOutline2 (3,,) [1 s]
  Passed SimpleScenarioOutline3 [1 s]
  Passed SimpleScenarioOutline4 [1 s]
Class Cleanup method Feature1Feature.FeatureTearDownAsync failed. Error Message: Expected featureData.TestRunners.Count to be 11 because One TestRunner for before/after hooks and one for each test is created, but found 1.. Stack Trace:     at FluentAssertions.Execution.LateBoundTestFramework.Throw(String message)
   at FluentAssertions.Execution.TestFrameworkProvider.Throw(String message)
   at FluentAssertions.Execution.DefaultAssertionStrategy.HandleFailure(String message)
   at FluentAssertions.Execution.AssertionScope.FailWith(String message, Object[] args)
   at FluentAssertions.Numeric.NumericAssertions`1.Be(T expected, String because, Object[] becauseArgs)
   at TraceSteps.AfterFeature(FeatureContext featureContext, ITestRunner testRunner) in temp\RR\R69c59efd\S50c277a4\TestProj_0920d173\BindingsClass_d2b01f76.cs:line 53
   at InvokeStub_Action`3.Invoke(Object, Span`1)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
--- End of stack trace from previous location ---
   at Reqnroll.Bindings.BindingInvoker.InvokeBindingAsync(IBinding binding, IContextManager contextManager, Object[] arguments, ITestTracer testTracer, DurationHolder durationHolder)
   at Reqnroll.Infrastructure.TestExecutionEngine.InvokeHookAsync(IAsyncBindingInvoker invoker, IHookBinding hookBinding, HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.FireEventsAsync(HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.FireEventsAsync(HookType hookType)
   at Reqnroll.Infrastructure.TestExecutionEngine.OnFeatureEndAsync()
   at Reqnroll.TestRunner.OnFeatureEndAsync()
   at TestProj_0920d173.Feature1Feature.FeatureTearDownAsync()

  Passed SimpleScenarioOutline5 [1 s]
  Passed SimpleScenarioOutline6 [1 s]
Results File: temp\RR\R69c59efd\S50c277a4\TestProj_0920d173\TestResults\obligaron_DESKTOP-CEF20VJ_2024-05-07_21_39_58.trx

Test Run Successful.
Total tests: 10
     Passed: 10
 Total time: 10,7621 Seconds
     1>Done Building Project "temp\RR\R69c59efd\S50c277a4\S50c277a4.sln" (VSTest target(s)).

Build succeeded.
    0 Warning(s)
    0 Error(s)

Time Elapsed 00:00:11.17


-> Process "dotnet" test --no-build --logger trx -v n "temp\RR\R69c59efd\S50c277a4\S50c277a4.sln" executed in 00:00:11.3473967 with ExitCode:0.
-> done: ExecutionSteps.WhenIExecuteTheTests() (16,4s)
Then the execution log should contain text 'Was parallel'
-> error: Expected containsAtAll to be true because either Trx output or program output should contain 'Was parallel'. Trx Output is: When I do something in Scenario 'Simple Scenario Outline 5'
Start index: 9, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 5'
Start index: 9, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 6'
Start index: 10, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 6'
Start index: 10, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 4'
Start index: 8, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 4'
Start index: 8, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 3'
Start index: 7, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 3'
Start index: 7, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 2'
Start index: 4, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 2'
Start index: 4, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 2'
Start index: 5, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 2'
Start index: 5, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 2'
Start index: 6, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 2'
Start index: 6, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 1'
Start index: 1, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 1'
Start index: 1, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 1'
Start index: 2, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 1'
Start index: 2, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
When I do something in Scenario 'Simple Scenario Outline 1'
Start index: 3, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)



TestContext Messages:
When I do something in Scenario 'Simple Scenario Outline 1'
Start index: 3, Worker: 15
Was not parallel
-> done: TraceSteps.WhenIDoSomething("Simple Scenario O...") (1,0s)
, but found False. (0,0s)
And the execution summary should contain
  --- table step argument ---
  | Total | Succeeded |
  | 10    | 10        |
-> skipped because of previous errors

2. The static feature test runner is problematic. We will need to find an alternative way, but I need a bit more time to give a suggestion.

Can you elaborate?
The ClassInitializeAttribute (BeforeFeature) and ClassCleanupAttribute (AfterFeature) are only allowed on static Methods. So to share state between these methods and to give the state to the per-scenario Runners, we need them or?

3. The "Specs" project will be repurposed to contain only specification related scenarios only, not technical tests. Currently it is only running with MsTest. The new place for such tests is the "SystemTests" project, but not all tests have been migrated yet to the new project. (Particularly the parallel execution tests not yet.) So these tests would need to be ported to SystemTests (e.g. to https://github.com/reqnroll/Reqnroll/blob/main/Tests/Reqnroll.SystemTests/Generation/MsTestGenerationTest.cs, but even better if you create a separate class for these.) This is a bit of extra work, but the SystemTests uses the same infrastructure, so the porting is not that horrible.

I put them on the same place where the other parallel execution tests are. 😀

Should all tests move together (in a separate PR) or should I move my test in this PR?
If I should move them, should I use the same logic as Sepcs Project and use .feature files or hard coded tests like in Reqnroll.SystemTests/Generation/MsTestGenerationTest.cs?
Note: The current tests could be reused if NUnit also supports parallel execution.

@gasparnagy
Copy link
Contributor

Thx. Super.

Can you elaborate? The ClassInitializeAttribute (BeforeFeature) and ClassCleanupAttribute (AfterFeature) are only allowed on static Methods. So to share state between these methods and to give the state to the per-scenario Runners, we need them or?

Got the question, but I need the deeper look to make anything more meaningful. In general, Reqnroll should be able to manage Before/After feature without the use of the class-level support from the frameworks, but I don't remember now how. (There must be a code somewhere that "realizes" that you are on a new feature and "close" the previous one and "start" the new one.)

I put them on the same place where the other parallel execution tests are. 😀 Should all tests move together (in a separate PR) or should I move my test in this PR?

Yeah... we have some legacy to work on. I don't have a strong preference whether it should be a separate PR or in this one. I think I would include it to this one, because then you have more flexibility for experimenting on how convenient they are in the current form.

I don't think we can do an "as-is" port of them to the new system, this is why I delayed those hoping for someone, like you to see them with a fresh mind... 😊. So feel free to move them and if you feel that they are not convenient or more complex/slow than it should be, feel free to change.

@Code-Grump
Copy link

Since I'm deep in all the test frameworks thanks to looking at code-generation, I think I should chip in here. The before-/after-feature hooks are definitely the most troublesome to get right, and in a world of parallel execution, trying to talk about them in terms of "previous" and "next" feature is going the wrong way. We have to imagine what happens if all the features were to be executed at the same time or entirely sequentially, or overlapped with each other, or interleaved.

I believe we can really only make two guarantees:

  • Before Feature will be executed some time before the any scenarios of that feature are invoked.
  • After Feature will be execute some time after all the scenarios of that feature have been invoked.

That might mean all of the "befores" run immediately at the start of test execution and all the "afters" run right before the test-runner terminates. Trying to control exactly when these things happen in relation to each-other is probably not beneficial, and for those trying to get highly parallelised execution it's actively harmful.

Which leads us to relying on the test-framework's hooks to signal when these events occur and dealing with the scope problems it produces, like having to resolve a separate test-runner for the feature-level scopes.

@gasparnagy
Copy link
Contributor

@Code-Grump Thx for the useful insight. That is another topic that we should have a discussion about. I tell you how it is intended to work currently:

  • Reqnroll somehow assigns the test execution requests to "test-threads". These are basically logical flows (not physical threads!) and Reqnroll ensures that the test execution within the same test-thread are not started parallel. (How it ensures that is out of scope here, and it might depend on the test runner framework, but this is given.) Of course in an unlucky situation it can happen that every scenario will be executed in a unique test-thread, so there is no guarantee that is is "optimal", but usually it is.
  • Because of that above, within a test-thread there IS a before/after logic between scenarios and this is what Reqnroll uses to decide on running the before/after feature hooks (if there is no better option received from the Test Runner)
  • This means that in a parallel execution situation:
    • the before/after feature of a particular feature can be executed multiple times, even parallel to each-other (e.g. if a scenario from that feature comes in in multiple test-threads)
    • even within a test-thread the before/after feature of a particular feature can be executed multiple times (if the test runner randomizes the scenarios across feature files - many does that)

This is the current behavior. From that, it is obviously visible that before/after feature hooks are senseless in a parallel execution environment, but they are also senseless if you run your tests (even single threaded) with a runner that randomizes the execution order.

I'm just highlighting this last paragraph, to indicate that we should not over-think the before/after feature hooks problem, because their usage is anyway meaningless in most of the current situations. In fact we should maybe make them obsolete in my opinion, but of course we then would need to have a story to provide backwards compatibility (e.g. make a sample with before/after scenario that simulates the before/after feature by remember the last feature).

So probably NOW we should have a solution that is convenient for us (not involving the magical class-level features of the runners) and announce them as obsolete. But let's not spend time on a "better" before/after feature hooks.

What do you think?

@obligaron
Copy link
Contributor Author

I don't think we can do an "as-is" port of them to the new system, this is why I delayed those hoping for someone, like you to see them with a fresh mind... 😊. So feel free to move them and if you feel that they are not convenient or more complex/slow than it should be, feel free to change.

I'll give it a try later. 🙂

Because of that above, within a test-thread there IS a before/after logic between scenarios and this is what Reqnroll uses to decide on running the before/after feature hooks (if there is no better option received from the Test Runner)

I have only looked at the MsTest code (yet). But when using MsTest the before/after feature hooks are not invoked by the scenario execution. Before/after feature is triggered by the ClassInitializeAttribute/ClassCleanupAttribute of MsTest. This means that MsTest invokes the methods and ensures that they are executed before/after all (scenario) tests of the class (feature file in our case) are executed or?

So from my understanding it should be possible to have a before/after feature implemented correctly (for MsTest at least). I tried to verify it with my added tests and it looks good. But I'm new to the Reqnroll codebase and maybe I'm missing something. So if you could give me a point where I could look at or debug to get more into the problem, I would be glad.

Which leads us to relying on the test-framework's hooks to signal when these events occur and dealing with the scope problems it produces, like having to resolve a separate test-runner for the feature-level scopes.

With this PR I have tried to implement this as you describe. There is a static TestRunner/TestExecutionEngine for the before/after feature execution and one TestRunner/TestExecutionEngine for each scenario execution (this instance gets the feature info / BoDi container passed from the static instance).

@gasparnagy
Copy link
Contributor

@Code-Grump @obligaron I had time to review a bit deeper the current before/after feature implementation and you are right. Actually that code that automatically triggers before/after feature without the need for calling in Before/After feature from generated code does not exists 😀. Maybe I was only dreaming of it or it was just a never-finished PR. I did a quick try now and it seems to be possible, but it depends on another issue (#123). I will take care of that, in the meantime please keep working with the current generation model (with the note that it will be simplified anyway soon hopefully).

So ignore my comment about the static test runner field for now, I still need to do the proper review of this PR.

Let's continue the whole before/after feature discussion topic at https://github.com/orgs/reqnroll/discussions/124.

@obligaron
Copy link
Contributor Author

Converting to draft for now, until

@obligaron obligaron marked this pull request as draft May 13, 2024 16:50
@obligaron obligaron mentioned this pull request May 27, 2024
10 tasks
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.

None yet

3 participants