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

Unexpected steps nesting when using async step helper's functions #383

Closed
1 of 3 tasks
delatrie opened this issue Sep 26, 2023 · 0 comments · Fixed by #393
Closed
1 of 3 tasks

Unexpected steps nesting when using async step helper's functions #383

delatrie opened this issue Sep 26, 2023 · 0 comments · Fixed by #393
Assignees

Comments

@delatrie
Copy link
Contributor

delatrie commented Sep 26, 2023

I'm submitting a ...

  • bug report
  • feature request
  • support request => Please do not submit support request here, see note at the top of this template.

What is the current behavior?

When a task-returning function of Allure.Net.Commons.Steps.CoreStepsHelper is called (either directly, or via NUnit.Allure.Core.StepsHelper or Allure.Xunit.Steps), the step/fixture it creates remains in context after the exit.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem

Let's consider the following example:

using NUnit.Allure.Core;
using NUnit.Framework;
using System.Threading.Tasks;

[AllureNUnit]
public class MyTestsClass
{
    [Test]
    public async Task MyTest()
    {
        await StepsHelper.Step("Step 1", async () =>
        {
            await Task.CompletedTask;
        });
        StepsHelper.Step("Step 2");
    }
}

Here two consecutive steps are expected, but nested steps are added instead:

What is the expected behavior?

All step/fixture creating functions must properly close the step/fixture they create including properly removing them from the context.

Please tell us about your environment:

  • Allure version: 2.23.1
  • Test framework: NUnit@3.13.3
  • Allure adaptor: allure-testng@2.10.0-preview.1

Other information

The overload of Step to create a step from an async function has the following definition in Allure.Net.Commons.Steps.CoreStepsHelper:

public static Task<T> Step<T>(string name, Func<Task<T>> action)
{
    StartStep(name);
    return Execute(action);
}

The Execute method has the following signature:

private static async Task<T> Execute<T>(Func<Task<T>>)

The key moment is that Step isn't an async method, while Execute is. That means the following sequence of actions happens when calling Step:

  1. The callee (a test method) calls Step. Since this is a plain method call, no execution context captured by the CLR at that point.
  2. StartStep modifies the callee's context by creating a new step.
  3. The modified context is captured by the CLR in a task created to run the Execute async method.
  4. When Execute completes, the CLR restores the context captured at step 3. This context becomes a new context of the callee. Note, that in this context the step still exists.

The same considerations apply to other methods that return Task instances. That should be easy to fix though: we just need to make every such method an async one. That makes the CLR capture the context earlier, at the point of a call. The restored context then will not hold the step/fixture.

@delatrie delatrie self-assigned this Sep 26, 2023
delatrie added a commit that referenced this issue Oct 3, 2023
  - make async Step, Before and After obey .NET's ExecutionContext capturing rules
  - refactor CoreStepsHelper to be more testable
  - write tests for CoreStepsHelper's Step, Before and After

More tests are needed to fully cover CoreStepsHelper.

Fixes #383
Fixed #388
delatrie added a commit that referenced this issue Oct 10, 2023
  - make async Step, Before and After obey .NET's ExecutionContext capturing rules
  - refactor CoreStepsHelper to be more testable
  - write tests for CoreStepsHelper's Step, Before and After

More tests are needed to fully cover CoreStepsHelper.

Fixes #383
Fixed #388
@delatrie delatrie linked a pull request Oct 10, 2023 that will close this issue
2 tasks
neparij pushed a commit that referenced this issue Oct 11, 2023
* allure-xunit: provide default config when allureConfig.json is missing

Fixes #381

* Fix async step/fixture wrapper functions

  - make async Step, Before and After obey .NET's ExecutionContext capturing rules
  - refactor CoreStepsHelper to be more testable
  - write tests for CoreStepsHelper's Step, Before and After

More tests are needed to fully cover CoreStepsHelper.

Fixes #383
Fixed #388

* Add async suffix to private step methods

This helps to make distinction more clear to a reader.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant