Fix tests so they run with VS test runner

[Porges]

• Added NUnit test adapter.
• Fixed paths to test certificates. (Previously it would look in the solution directory, not the test project output.)

[jnm2]

I keep the NUnit test adapter installed for VSTest and usually download it and to use with VSTest in the build script. It always felt cleaner to me since I think of that as a build-time detail. What do you think of that option?

I'm surprised about the paths too, since I ran VSTest for all tests while writing them. If I recall correctly, it's looking in the output folder because the test certificates are marked Copy to Output Directory.

[Porges]

I think you should be able to clone the repo, open the solution and run the tests straight away without any manual intervention :grin: This way also has the advantage that the exact adapter version is specified so you're guaranteed to get the version that matches what other developers are using.

I'm not exactly sure what you mean by installing the adapter in the build script. Do you mean installing the .vsix? I prefer not to pollute my extensions list with per-project test runners..

On the paths, I'm not sure why they were incorrect – what I was seeing before was that the working directory when the tests were run was set to the solution directory. Perhaps this is a difference between test runners... either way, I think using NUnit's TestContext class should ensure that this is consistent.

[jnm2]

Fair enough, it does make it easier to get up and running in the IDE for those without ReSharper or NCrunch. I've been around projects where this isn't the case, but they do have a build script which runs tests out of the box and we don't have that here.

Do you mean installing the .vsix?

Not exactly, I mean this, where .\tools is at the root of the repo and gitignored:

nuget install NUnit3TestAdapter -outputdirectory tools -excludeversion
vstest.console.exe <assemblies> /TestAdapterPath:tools

Or the Cake equivalent:

NuGetInstall("NUnit3TestAdapter", new NuGetInstallSettings { OutputDirectory = buildToolsDir, ExcludeVersion = true });
VSTest(testAssemblies, new VSTestSettings { TestAdapterPath = buildToolsDir });

I prefer getting the latest runner rather than having to go to the NuGet package manager to become aware of updates. All developers will be on the same version this way. If they use the IDE, VS keeps the VSIX updated; if they use the build script, the latest is downloaded. But- this is personal preference. Lowering the barrier to entry for new contributors wins. :+1:

[jnm2]

Just tested a fresh clone, sure enough the paths are messed up with VSTest. Thanks for fixing that!

[mike-barnett]

I'm happy to go with whatever version of C# you all like. But until VS2017 is a little older I would prefer to require at most VS2015. If you tell me where to set the language level (or feel free to submit a PR) then we can do that.

[jnm2]

@mike-barnett It's a matter of adding <LangVersion>6</LangVersion> to the main property group of each csproj. https://github.com/Microsoft/ILMerge/pull/8

[Porges]

Would there be any objection to updating to Framework 4.5.2 at the same time? It's the lowest supported version now (and would allow use of WeakReference<T>).

[jnm2]

That sounds interesting! What would you do with WeakReference?

[Porges]

Replace the existing non-generic WeakReference 😉

[mike-barnett]

Okay, so are you all agreed on this change? Can I go ahead and merge the PR?

[jnm2]

All looks good to me!

[mike-barnett]

I ended up accepting another PR that caused merge conflicts with this. Can you fix them please?

[jnm2]

Hey @Porges I'd like to use your changes in master. Can I help rebase or do you have this under control?

[Porges]

Sorry about the delay, have been a bit distracted. I can fix this up for Tuesday next week (I'm away over the weekend), or feel free to try it yourself if you need it earlier 🙂

[jnm2]

Tuesday is good! Thanks so much!

[Porges]

Updated. However since someone else fixed this on master, the PR is really just "Update to Framework 4.5.2" now...

[jnm2]

Oh, I see @natemcmaster copied your changes in #10 which just got merged.