Skip to content

Inherited Test SetUp, TearDown, etc. are not executed in .NET Core if they are not public #2448

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

Closed
declancodes opened this issue Sep 18, 2017 · 9 comments

Comments

@declancodes
Copy link

declancodes commented Sep 18, 2017

Hello All,

In trying to port some NUnit tests to .NET Core I've found that methods with [OneTimeSetUp], [SetUp], [TearDown], and [OneTimeTearDown] attributes from a base class are not being called on [Test]s in derived classes. This is only occurring when running against .NET Core, for all versions of NUnit I've tried (3.7.1 and up).

Apologies if this has been documented elsewhere (I believe I checked thoroughly...) or if I'm doing something silly here!

To reproduce:
csproj file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>netcoreapp2.0</TargetFramework>
  </PropertyGroup>

  <!--<PropertyGroup>
    <TargetFramework>net46</TargetFramework>
  </PropertyGroup>
  <PropertyGroup Condition="'$(TargetFramework)' == 'net46'">
    <TargetFrameworkIdentifier>.NETFramework</TargetFrameworkIdentifier>
  </PropertyGroup>-->

  <ItemGroup>
    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="15.3.0" />
    <PackageReference Include="NUnit" Version="3.8.1" />
    <PackageReference Include="NUnit3TestAdapter" Version="3.8.0" />
  </ItemGroup>

</Project>

Test:

public class BaseClass
    {
        // These methods all get called when running the "Inheritance" test in .NET framework,
        // but not in .NET Core
        [OneTimeSetUp]
        protected void OneTimeSetUp()
        {
            Console.WriteLine("OneTimeSetUp");
        }

        [SetUp]
        protected void SetUp()
        {
            Console.WriteLine("SetUp");
        }

        [TearDown]
        protected void TearDown()
        {
            Console.WriteLine("TearDown");
        }

        [OneTimeTearDown]
        protected void OneTimeTearDown()
        {
            Console.WriteLine("OneTimeTearDown");
        }

        public class DerivedClass : BaseClass
        {
            [Test]
            public void Inheritance()
            {
                Console.WriteLine("Test");
            }
        }
    }

EDIT: use XML, not C#, formatting for csproj

@rprouse
Copy link
Member

rprouse commented Sep 18, 2017

Thanks, I think I've seen this issue in another repository, but it does belong here. Your code looks good, so I am going to mark this as a bug immediately. The reflection APIs have subtle differences between .NET Core and .NET Framework, so it is likely caused by that. We will look into it. Thanks.

@mikkelbu
Copy link
Member

Hi @declancodes. Can you check wether making the methods public solves the problem (see #1657 for a similar issue).

@declancodes
Copy link
Author

Changing those methods to public makes them called as expected - thanks for the workaround, @mikkelbu

Sounds like this is the same issue, but #1657 is muddied by the fact that it references the dotnet-test-nunit repo in the title. I'll leave the decision of whether to close this one up to the maintainers

@CharliePoole
Copy link
Member

CharliePoole commented Sep 19, 2017

If non-public setup and teardown are working in other builds, I would say there's some kind of bug here. At a minimum it should be documented.

@rprouse rprouse changed the title Inherited Test SetUp, TearDown, etc. are not executed in .NET Core Inherited Test SetUp, TearDown, etc. are not executed in .NET Core if they are not public Sep 19, 2017
@rprouse
Copy link
Member

rprouse commented Sep 19, 2017

I have updated the title to highlight the problem. I also think this issue is more clearly documented than #1657, so I am going to close the other issue as a duplicate.

@mikkelbu
Copy link
Member

Just looking at the code, I think the problem lies in TypeExtensions.GetAllMethods which looks as follows:

        static IList<MethodInfo> GetAllMethods(this Type type, bool includeBaseStatic = false)
        {
            List<MethodInfo> methods = type.GetTypeInfo().DeclaredMethods.ToList();
            type = type.GetTypeInfo().BaseType;
            if (type != null)
            {
                var baseMethods = type.GetAllMethods(includeBaseStatic)
                    .Where(b => b.IsPublic && (includeBaseStatic || !b.IsStatic) && !methods.Any(m => m.GetRuntimeBaseDefinition() == b));
                methods.AddRange(baseMethods);
            }

            return methods;
        }

We only look at the methods of the base-class which have IsPublic == true. That seems wrong.

I'll try to add some tests and supply a PR.

mikkelbu added a commit that referenced this issue Sep 30, 2017
We now return all non-private methods from base classes
instead of only all public-methods. Furthermore, when
filtering with BindingFlags.NonPublic we return all
non-public methods instead of all private methods.

Tests have been added to ensure the behaviour is the same
between regular .NET reflection and our own reflection
methods.

Also the test `CanGetStaticMethodsOnBase()` has been
corrected and enabled.

Fixes #2448
@mikkelbu mikkelbu self-assigned this Sep 30, 2017
@rprouse
Copy link
Member

rprouse commented Oct 1, 2017

Good catch, I suspected it was in our extensions, but didn't know which one.

@mikkelbu
Copy link
Member

mikkelbu commented Oct 1, 2017

I think that there is a similar problem for properties (the code in ApplyBindingFlags wrt. BindingFlags.NonPublic is quite similar). I'll create a new issue for this.

@ChrisMaddock
Copy link
Member

@rprouse - I haven't milestoned this, as I'm not sure what you'll be doing for 3.8.2. (With hindsight - maybe I shouldn't have merged either - sorry!)

Will leave it with you.

@rprouse rprouse added this to the 3.9 milestone Oct 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants