Nunit-console: Dogfood NUnit.Analyzers via the nunit-console tests

[ChrisMaddock]

I'd like to add the nunit-analyzers NuGet package to this project, in part to dog-food the analyzers, and in part to aid contributors here in following good practice.

I took a look at this - fortunately our tests don't flag up any of the 'error-catching' analyzers! We do however flag the analyzer designed to direct users to the constraint syntax, rather than the classic syntax. There are 221 usages of classic syntax in the nunit-console repo tests.

We have two options here:

1. Auto-Convert our classic syntax tests to use constraint syntax.
2. Suppress these analyzers to either "Information" or "Hidden" levels, rather than "Error".

Out of these two - I would favour option 1. The NUnit team generally promotes the constraint sytnax over the classic. (Or we do through our lack of updates for classic, if not so explicitly!) My opinion is that our own tests should be an example of 'good practice' with NUnit - and moving to the constraint syntax is a part of that, in my opinion.

I'll throw up a PR with what the changes will look like shortly - that should affect the decision we make here, however - it's all automated and easily revertible.

[CharliePoole]

If we continue to build the classic assertions, don't we need to continue to test them?

[ChrisMaddock]

Absolutely - but surely that's the responsibility of the framework tests, not the engine/console tests?

In the framework - I would agree that we would want to suppress those analyzers. But the framework also hits several of the known bugs currently in the analyzers project, and is a much bigger test suite in general - hence starting here. 🙂

[CharliePoole]

No sweat... I didn't notice what repo we were in.

[rprouse]

Personally I like the idea of dogfooding the analyzers and don't see any real downside other than the work of updating our tests to the new syntax.

[mikkelbu]

I took a look at this - fortunately our tests don't flag up any of the 'error-catching' analyzers!

The plan is also that the 'error-catching' analyzers are for situations that will cause run-time errors, like a TestCaseSource that refers to a non-static field, property or method, and similar situations.

[mikkelbu]

I think it is fine to add the analyzer to this project.
My only regrets are that we have not finished all the issues in the Release 0.2 milestone (please don't look at the time schedule 😄) such that we covered most of the regular issues in Nunit project. This is also the reason why we don't have an "official" nuget package yet, so we need to refer to myget and also run into the problem that the version number currently is bumped by each commit to master, hence the
<PackageReference Include="NUnit.Analyzers" Version="0.1.0-dev-00079" /> in the project files.