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

Add support for async NUnit tests #314

Merged
merged 2 commits into from
Dec 30, 2022

Conversation

overlord
Copy link
Contributor

@overlord overlord commented Dec 26, 2022

This pull request provides workaround or maybe even fixes the issues (in Allure.NUnit 2.9.2-preview.1) with async test methods in NUnit.Allure.

It fixes the case when async methods marked with [AllureStep] like the following:

[AllureStep("...")]
public async Task EnsureDeletedStep(string itemCode)
{ ... }

fails in runtime with exception like the following:

System.ArgumentNullException : Value cannot be null. (Parameter 'key')
---
Stack Trace:
ThrowHelper.ThrowArgumentNullException(String name)
ConcurrentDictionary`2.TryGetValue(TKey key, TValue& value)
ConcurrentDictionary`2.get_Item(TKey key)
AllureStorage.Get[T](String uuid)
AllureStorage.AddStep(String parentUuid, String uuid, StepResult stepResult)
AllureLifecycle.StartStep(String parentUuid, String uuid, StepResult stepResult)
AllureLifecycle.StartStep(String uuid, StepResult result)
AllureStepAspect.WrapStep(String name, MethodBase methodBase, Object[] arguments, Func`2 method)
...

The main problem was in the ThreadStatic field AllureStorage.stepContext - that fails on cross-thread execution of async methods.

I have tried to fix it and have provided some unit tests for new async behaviour (see. Allure.NUnit.AsyncTests.AsyncLifeCycleTests)

@neparij
Copy link
Contributor

neparij commented Dec 29, 2022

Hello, @overlord !
Thank you for contributing to Allure!
I'll look your PR and provide the feedback soon.

Looks like it solves this issue completely.

Copy link
Contributor

@neparij neparij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice solution, could you look at review?

Allure.NUnit.AsyncTests/Allure.NUnit.AsyncTests.csproj Outdated Show resolved Hide resolved
Allure.NUnit.AsyncTests/AsyncLifeCycleTests.cs Outdated Show resolved Hide resolved
Allure.NUnit.AsyncTests/AsyncOneTimeSetup.cs Outdated Show resolved Hide resolved
Allure.NUnit/Core/AllureExtensions.cs Outdated Show resolved Hide resolved
allure-csharp.sln Outdated Show resolved Hide resolved
Allure.NUnit.AsyncTests/AsyncOneTimeSetup.cs Outdated Show resolved Hide resolved
@overlord overlord requested review from neparij and undron and removed request for neparij and undron December 29, 2022 18:58
Copy link
Contributor

@neparij neparij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove accidentally added file please.

.editorconfig Outdated
@@ -0,0 +1,24 @@
# EditorConfig is awesome: http://EditorConfig.org
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs to be removed

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

Successfully merging this pull request may close these issues.

4 participants