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

Snapper Nunit path resolver can fail #17

Closed
JBrejnholt opened this issue Apr 23, 2019 · 4 comments
Closed

Snapper Nunit path resolver can fail #17

JBrejnholt opened this issue Apr 23, 2019 · 4 comments

Comments

@JBrejnholt
Copy link

JBrejnholt commented Apr 23, 2019

We have a scenario where we package some of our unit tests, which use Snapper, as a nuget to be distributed and consumed.

When doing this, we see an exception thrown by the Snapper Nunit path resolver code, which tries to combine path elements, but fails since it is using StackFrame.GetFileName() which returns null. Actually, this method documents that it can return null.

The code we are referring to is in NUnitTestHelper.cs:

 public static (MethodBase, string) GetCallingTestInfo()
        {
            var stackTrace = new StackTrace(2, true);
            foreach (var stackFrame in stackTrace.GetFrames() ?? new StackFrame[0])
            {
                var method = stackFrame.GetMethod();

                if (IsNUnitTestMethod(method))
                    return (method, stackFrame.GetFileName());

                var asyncMethod = GetMethodBaseOfAsyncMethod(method);
                if (IsNUnitTestMethod(asyncMethod))
                    return (asyncMethod, stackFrame.GetFileName());
            }

            throw new InvalidOperationException(
                "Snapshots can only be created from classes decorated with the [Test] attribute");
        }

So, if stackFrame.GetFileName() returns null, the caller - NUnitPathResolver.ResolvePath() - tries to get its directory using Path.GetDirectoryName(), which itself will return null. The null becomes the first parameter to Path.Combine(), which throws an ArgumentNullException.

As we see it, the only reason to use StackFrame.GetFileName() is to obtain the directory of the actual unit test assembly. But this could be done instead by using the declaring type of the unit test method found, and getting its assembly. So, the change would replace:

return (method, stackFrame.GetFileName());

with

return ( method, method.DeclaringType.Assembly.Location );

...and the same for the async case. Or even put the Path.GetDirectoryName() at this level instead of the caller, since only the directory is relevant.

What do you think?

@theramis
Copy link
Owner

Hey @JinhongZh,

Thats an interesting use case. I did a quick test using method.DeclaringType.Assembly.Location rather than StackFrame.GetFileName() and I found that it unfortunately won't work.

method.DeclaringType.Assembly.Location returns the location of the dll which is the compiled code. Snapper needs to store the snapshots next to the code file not the generated dll.

Would it be possible for you to make an example project where this issue occurs and I can use that to figure out a solution.

@JBrejnholt
Copy link
Author

Hi @theramis ,
Thanks for your swift reply :)
Yes, our use case is a bit special. We are trying to deploy a set of services in different environment, and we want to run the same snapshot tests against the services in those environments. Due to there are some parameters need to be updated accordingly, we have a PowerShell script to update the settings file first, and then execute the snapshot tests via "dotnet vstest XXTest.dll". Those all worked fine, until in one of the projects, where the setup was done in C# code, to keep the same workflow as it always do, we tried to execute the PowerShell script from the there, and then we hit the problem where the StackFrame.GetFileName() didn't work.
For packaging the snapshot test project nuget, we simply take the output from the dotnetcore publish folder and insert them as contentFiles (nuspec file), in this way we can inspect the folder layout and use the files directly when the nuget is been installed in the new project.
I will try to make an example project as you suggested, and let you know when it is ready.
Thanks :)

@JBrejnholt
Copy link
Author

Hi again,
I tried to created two example projects to reproduce the problem we are facing, and then I found there were a problem when directly using the PowerShell class from System.Management.Automation nuget. The discovery made me think the way of executing the dll via Powershell might not be the best fit in this case. After I modified the project to use cmd.exe to execute the snapshot test dll, the problem went away.

I don't think there is a need to dig further in this special usage of the Snapper.

Thanks for the great tool by the way :)

@fgather
Copy link
Contributor

fgather commented Apr 25, 2019

Hi,

I've also run in this problem. It might be related to this:
dotnet/coreclr#14696

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

No branches or pull requests

3 participants