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

The code inside AfterTestRun fails to complete before the tests are unloaded #1348

Open
godrose opened this issue Nov 23, 2018 · 45 comments

Comments

10 participants
@godrose
Copy link

commented Nov 23, 2018

SpecFlow Version:

  • 3.0.150-beta

Used Test Runner

  • xUnit 2.4.1

Visual Studio Version

  • VS 2017 Enterprise

Are the latest Visual Studio updates installed?

  • Yes

.NET Framework:

  • .NET Core 2.0

Test Execution Method:

  • Resharper 2018

<SpecFlow> Section in app.config

{
"bindingCulture": {
"language": "en-us"
},
"language": {
"feature": "en-us"
},

"plugins": [],
"stepAssemblies": [
{
"assembly": "BDD.NetCore.Steps.Adapters"
},
{
"assembly": "BDD.NetCore.Launcher"
}
]
}

Repro Project

https://github.com/godrose/specflow-failing-after-test-run

Issue Description

The [AfterTestRun]-decorated method is not invoked. This does not work in the previous 3.0.0 beta release either.

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Nov 23, 2018

@godrose Could you provide us a complete project to reproduce this? Our integration tests are green for AfterTestRun hook.

@godrose

This comment has been minimized.

Copy link
Author

commented Nov 23, 2018

@SabotageAndi Here you go:
https://github.com/godrose/specflow-failing-after-test-run
Happens for both beta versions on Windows and MacOS
FYI, all other hooks are invoked successfully

@godrose

This comment has been minimized.

Copy link
Author

commented Nov 23, 2018

@SabotageAndi I might be wrong but it seems like you don't actually test the AfterTestRun hook
https://github.com/techtalk/SpecFlow/blob/master/Tests/TechTalk.SpecFlow.Specs/Features/Hooks/HookSupport.feature

@SamJongenelen

This comment has been minimized.

Copy link
Contributor

commented Nov 26, 2018

@SabotageAndi I might be wrong but it seems like you don't actually test the AfterTestRun hook
https://github.com/techtalk/SpecFlow/blob/master/Tests/TechTalk.SpecFlow.Specs/Features/Hooks/HookSupport.feature

I think you're right; since NUnit is being used, the test will never pass if commented out.

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Nov 26, 2018

@godrose & @SamJongenelen: It was a long day, so I am not sure if I oversee something.
Why do you think, we don't test the AfterTestRun hook?

Here is the test:
image

@godrose

This comment has been minimized.

Copy link
Author

commented Nov 26, 2018

@SabotageAndi @SamJongenelen Have you seen the repo? The issue is reproduced quite well there.

@godrose

This comment has been minimized.

Copy link
Author

commented Nov 26, 2018

@SabotageAndi Where are these tests located?

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Nov 27, 2018

I looked now at the repo and send you a PR godrose/specflow-failing-after-test-run#1 to better see, that the hooks are executed.
Yes, the AfterTestRun hook is executed.

The reason you don't see it, is that xUnit doesn't capture Console.Writeline. So you don't see your Console.WriteLine in the output.
Additionally it looks like if you debug it, the test process is stopped, also if you are on a break point.

So I changed the hook to write into a file. Now you see that the hook is executed.
image

About the tests:
They are these: https://github.com/techtalk/SpecFlow/blob/master/Tests/TechTalk.SpecFlow.Specs/Features/Hooks/HookSupport.feature
We generate with a generator plugin (https://github.com/techtalk/SpecFlow/tree/master/Tests/TechTalk.SpecFlow.Specs.Generator.SpecFlowPlugin) scenarios for the multiple configurations.
You have to build one time the solution that you see the tests.

@SamJongenelen

This comment has been minimized.

Copy link
Contributor

commented Nov 27, 2018

@godrose

This comment has been minimized.

Copy link
Author

commented Nov 28, 2018

@SabotageAndi Thanks. I'm checking the real project where this issue has happened. Interestingly enough, when I run the tests via dotnet test everything works as expected. The issue only is reproduced when I use Resharper.
I guess the issue can be closed now...

P.S. What I found out after some digging is the following:
If you add some waiting inside the AfterTestRun and then try to call the log line it doesn't work.
Again, I use Resharper and run tests from within VS. Sometimes I can actually see the breakpoint being hit but then the tests run is stopped. It looks like there's some race condition here...

@godrose godrose closed this Nov 28, 2018

.NET Core Support automation moved this from Todo to Done Nov 28, 2018

@godrose godrose reopened this Nov 28, 2018

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Nov 28, 2018

The AfterTestRun hook is a little bit special, because in the unit test runners there aren't any events that we can use.
So we are using the AssemblyUnload event.

From https://specflow.org/documentation/Hooks/:

Note: As most of the unit test runners do not provide a hook for executing logic once the tests have been executed, the [AfterTestRun] event is triggered by the test assembly unload event. The exact timing and thread of this execution may therefore differ for each test runner.

Perhaps this makes problems for you.

@godrose

This comment has been minimized.

Copy link
Author

commented Nov 28, 2018

Yes. This is the problem here. This used to work in the latest 2.x version which used netframework, though...
What could be the workaround here?

@godrose godrose changed the title AfterTestRun is not called The code inside AfterTestRun fails to complete before the tests are unloaded Nov 28, 2018

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Nov 28, 2018

ok, this would be possible. But it needs a lot of changes.
Currently we can't generate a class that has nothing to do with a feature file.

@gasparnagy Good ideas how we do this?

@nvborisenko

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2018

This issue is reproducible for me too. Target framework is netcore. I use MsTest framework as unit provider. The issue is not reproducible if change target framework to full net. Will come back to you soon with simple example to reproduce it. Please move this issue to ToDo list. Thx.

@nvborisenko

This comment has been minimized.

Copy link
Contributor

commented Dec 19, 2018

This is simple example.
Issue1348.zip

Unpack it and just execute tests via dotnet test. Test project is multi-targeted: net471 and netcoreapp2.1. Command executes tests against 2 target frameworks. We have [BeforeTestRun] and [AfterTestRun] hooks which writes some text message to bin\debug\log.txt file.

We can see that [AfterTestRun] hook is not awaited in netcoreapp2.1 framework, and works good in net471 framework. If you remove Thread.Sleep() in hook and execute tests again, we will see correct text message in the file.

I can suppose that actually [AfterTestRun] hook is executed, but not awaited for completion in netcore tests.

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Dec 20, 2018

I am not sure if Assembly unloading is already implemented in .NET Core ...
@nvborisenko Did you try it also with .NET Core 2.2?

@nvborisenko

This comment has been minimized.

Copy link
Contributor

commented Dec 20, 2018

Even with .Net Core 3.0-preview, doesn't work with vstest. Raised issue to vstest repository.

@godrose

This comment has been minimized.

Copy link
Author

commented Jan 8, 2019

@SabotageAndi Any update on this issue? Is it planned to be a part of 3.0.0 release?

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Jan 8, 2019

No idea tbh. I am the second day into the office after vacation and have now read the VSTest issue.
I have to think how to solve this.

@nvborisenko

This comment has been minimized.

Copy link
Contributor

commented Jan 11, 2019

As to me AssemblyCleanup per one assembly is expected.

What about single code file generation: may be it can be resolved by msbuild props and targets. For example if my project references to "SpecFlow.NUnit" package, and I want to build my project, then target from SpecFlow.NUnit package adds file for compilation on "prepare for build" phase. I hvae no big experience here.. just thoughts.

@StevenBlair123

This comment has been minimized.

Copy link

commented Jan 14, 2019

We have been experiencing this problem. Hoping someone can come up with a solution.

We are using:

xUnit
net core 2.0
SpecFlow.xUnit 3.0.161-beta

AfterTestRun is not being fired.
We tried running with our debugger, and occasionally, the source file containing the AfterTestRun method is shown, but immediately the process stops.

@StuartFergusonVme

This comment has been minimized.

Copy link

commented Jan 29, 2019

Done a bit more investigation on this issue and have confirmed as the issue suggests that the AfterTestRun method is indeed being fired but the code in this does not complete :(

Added in a foreground thread starting in BeginTestRun in an effort to keep the process from being unloaded but this does not seem to help, in fact it seems to hinder the issue as the AfterTestRun stops being called.

Also added the thread code into AfterTestRun to try and allow the code in there to complete but again the process is getting killed before this can completed.

@ninjaboy

This comment has been minimized.

Copy link

commented Jan 30, 2019

hi guys, we are using xUnit, .netcore2.2, specflow.xunit 3.0.161-beta as well
Can confirm that AfterTestRun is not running neither by using dotnet test, nor by running the tests in VS Tests runner in debug.
I can also confirm that I am certain that it is not running because there are some processes to be closed in the AfterTestRun which remain hanging.

@StuartFergusonVme

This comment has been minimized.

Copy link

commented Jan 31, 2019

@ninjaboy I did a decent amount of testing on this the other day and the method is getting fired consistently BUT the time the method gets to complete appears to be very short which is why any cleanup code you are doing in there is not getting done.

I proved this by adding a line of trace logging similar to the following

sw.WriteLine("In After Test Run")
// Do a clean up task
sw.WriteLine("Clean up task 1 done")

I could see the "In After Test Run" line written to my log file consistently. However never got the "Clean up task 1 done" log entry.

Is any one from the SpecFlow team looking at this issue as this is causing many people serious issues as shared clean up code is not getting run under .net core.

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Jan 31, 2019

We have it on our backlog, but didn't had yet the time for it. So feel free to send us PRs to fix this.

And I still have no idea how to solve this. see my comment #1348 (comment). So if somebody has a brilliant idea, I am open for suggestions.

In the Issue microsoft/vstest#1873 @nvborisenko opened, the VSTest teams explains the behaviour. They kill it after 100ms.

@godrose

This comment has been minimized.

Copy link
Author

commented Feb 4, 2019

Hi, everyone.
I managed to find a working solution for xUnit.
Disclaimer: It's neither clean nor generic but it works and can be made clean and generic and integrated into SpecFlow.
These are the steps:

  • Update to the 3.0.169-beta version of the packages
  • Add the bridge class to hook into real AfterAllTestsHaveBeenRunAndTheAssemblyIsAboutToBeUnloaded event:
    [CollectionDefinition("All")]
    public class AssemblyCleanupBridge : ICollectionFixture<AssemblyCleanupBridge>, IDisposable
    {
        void IDisposable.Dispose()
        {
            ScenarioContext.Current.Get<ILifecycleService>().Teardown();
        }
    }
  • Decorate the features that require cleanup with the @xunit:collection(All) attribute
  • Sit back and enjoy
@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Feb 4, 2019

I think, this would be possible to add to the code- generator.

@narcis-pv

This comment has been minimized.

Copy link

commented Feb 5, 2019

Another temporary workaround. Place the file bellow anywhere in your Acceptane Tests projects.
Does the unloading when there are no more features to run

using System.Collections.Generic;
using Willow.Test.Acceptance;
using Xunit;
using Xunit.Abstractions;

[assembly: CollectionBehavior(DisableTestParallelization = true)]
[assembly: TestCollectionOrderer(
    CollectionOrderer.TypeName,
    CollectionOrderer.AssemblyName)]

namespace Your.Test.Acceptance
{
    using System.Linq;
    using StepDefinitions;
    using TechTalk.SpecFlow;

    [Binding]
    public class CollectionOrderer : ITestCollectionOrderer
    {
        public const string TypeName = "Your.Test.Acceptance.CollectionOrderer";

        public const string AssemblyName = "Your.Test.Acceptance";

        private static int _currentSessionScenarios = 0; 

        public IEnumerable<ITestCollection> OrderTestCollections(
            IEnumerable<ITestCollection> testCollections)
        {
            _currentSessionScenarios = testCollections.Count();

            return testCollections;
        }

        [AfterFeature]
        public static void ScenarioTeardown()
        {
            _currentSessionScenarios--;

            if (_currentSessionScenarios <= 0)
            {
                ScenarioContext.Current.Get<ILifecycleService>().Teardown();
            }
        }
    }
}

@SabotageAndi SabotageAndi moved this from Done to Todo in .NET Core Support Feb 20, 2019

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Mar 25, 2019

The release of SpecFlow 3 is near. We (TechTalk Team) will not have time to fix this before.

@narcis-pv

This comment has been minimized.

Copy link

commented Mar 25, 2019

@SabotageAndi Honestly, this seems like a MAJOR issue. Better not release it, then release with this kind of broken behavior.

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Mar 25, 2019

It will go to the known issues (FYI: @Stephen-Mc).
We have to release because of the VS 2019 release, which happens on April 2nd. Can't change that.

The problem exists "only" for .NET Core. Full Framework is still working. And if you need more than 100 ms.
A Workaround also exists. Do the cleanup before the TestRun, so that you are sure to have a clean environment.

And for NUnit and MSTest we still have no idea how to solve this. Had anybody a flash of inspiration yet?

But SpecFlow is open source. We are happy to accept PR that fixes this issue.

@nvborisenko

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

For NUnit: https://github.com/nunit/docs/wiki/SetUpFixture-Attribute

A SetUpFixture outside of any namespace provides SetUp and TearDown for the entire assembly.

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Mar 25, 2019

@nvborisenko Do you know if there can be more than one class without namespace with the attribute?

@nvborisenko

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

Yes.
NUnitTestProject3.zip

@nvborisenko

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

For MSTest: https://www.meziantou.net/2018/02/12/mstest-v2-test-lifecycle-attributes

UnitTestProject2.zip

And cannot define more that one [AssemblyInitialize] attribute.

Actually cannot understand why Specflow generator requires more than one definition of setup attribute.

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Mar 25, 2019

@nvborisenko SpecFlow doesn't require multiple setup attributes.
But there are users out there, that are using the test frameworks attributes already and mix them with hooks.
When we now create a new class with them, two are defined and I have no idea what the test runners are doing.

@nvborisenko

This comment has been minimized.

Copy link
Contributor

commented Mar 25, 2019

Now I see. In my opinion SpecFlow should work as documented. Hooks should work by default, because of by default clients use SpecFlow feature files as tests "container". But for whom, who mixes feature files and unit tests "container", SpecFlow should leave a comment how to combine several invocations of setup methods into one if unit framework doesn't support multiple setup methods (MsTest). Rare case... Hence SpecFlow generator should take a control.

When we now create a new class with them, two are defined and I have no idea what the test runners are doing.

It can be easily verified, ask me to help to verify runners behavior. In any case, if setup methods of unit frameworks don't work as documented, it's a bug on runner side.

Adding:
Regarding versions, as for me it doesn't matter which SpecFlow release the fix should be included in, in 3.0 or 3.0.1. But this is high priority issue.

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Mar 28, 2019

I thought about how this could be implemented.

For getting the additional file into the project, I would create an additional MSBuild task, that add the file to the Compile- ItemGroup.
If possible, I would write the file to the obj- Folder.

The xUnit Code- Generator (https://github.com/techtalk/SpecFlow/blob/master/TechTalk.SpecFlow.Generator/UnitTestProvider/XUnitTestGeneratorProvider.cs & https://github.com/techtalk/SpecFlow/blob/master/TechTalk.SpecFlow.Generator/UnitTestProvider/XUnit2TestGeneratorProvider.cs) need the additional code for the collection definition.

One thing I don't know yet, is how to get a test runner instance without reflection or ScenarioContext.Current. But I will have a look at it in the next days.

So if someone wants to create a PR for this, you have now a starting point to do it.

@gasparnagy

This comment has been minimized.

Copy link
Collaborator

commented Mar 29, 2019

@SabotageAndi yes, this is a good idea: see #1458 for an easy way to ensure that the AfterTestRun hooks are called.

.NET Core Support automation moved this from Todo to Done Apr 5, 2019

SabotageAndi added a commit that referenced this issue Apr 5, 2019

Preparation to fix #1348: Expose TestRunnerManager.OnTestRunEnd to be…
… able to call it from plugins (#1458)

In order to fix #1348 in a way as @SabotageAndi suggested, we need an easy way to ensure that the AfterTestRun hooks fired even before AppDomain unload. 

This PR exposes an `OnTestRunEnd` method on the static API of `TestRunnerManager` to support this. The `OnTestRunEnd` method is safe to call multiple times (or call it before domain unload), it will fire the events only once. 

## Types of changes

- [ ] Bug fix (non-breaking change which fixes an issue).
- [x] New feature (non-breaking change which adds functionality).
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected).

## Checklist:

- [x] I've added tests for my code.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have added an entry to the changelog
@gasparnagy

This comment has been minimized.

Copy link
Collaborator

commented Apr 5, 2019

@SabotageAndi the code to generate the necessary calls for xunit is still have to be added. Shall we keep this issue open until that has been done?

@SabotageAndi SabotageAndi reopened this Apr 5, 2019

@SabotageAndi

This comment has been minimized.

Copy link
Collaborator

commented Apr 5, 2019

The close what not on purpose. Sometimes GitHub automations work to good. ;-)

I would left this issue open as long as it works for the 3 frameworks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.