nunit / dotnet-test-nunit

Deprecated test runner for .NET Core. For use with project.json only. For recent releases of .NET Core, use the NUnit 3 Visual Studio Adapter
https://www.nuget.org/packages/dotnet-test-nunit/
MIT License
67 stars 25 forks source link

Create an NUnit template for dotnet new #57

Closed rprouse closed 8 years ago

rprouse commented 8 years ago

dotnet/cli#1275 adds the command dotnet new -t xunittest to create a new XUnit test project. This makes NUnit a second class citizen in the dotnet ecosystem and gives XUnit a favoured position which I do not like. We should submit a PR to https://github.com/dotnet/cli to add NUnit.

To do so, we need to add a project template to https://github.com/dotnet/cli/tree/rel/1.0.0/src/dotnet/commands/dotnet-new

The initial commit for XUnit is https://github.com/dotnet/cli/commit/c066ef15404b840000bb64f0676b359d483bb31d

jplebre commented 8 years ago

not exactly sure I did this right, but please let me know if you have feedback ^^

rprouse commented 8 years ago

@jplebre thanks for the help, looks good so far. Let's hope the CLI team accepts it :smile:

jplebre commented 8 years ago

All good, just wanted to give something back to the community :)

jcansdale commented 8 years ago

Good stuff. :smile:

To what extent to we encourage the use of [SetUp]? Should it be promoted as normal and be part of the template?

jplebre commented 8 years ago

Good question.

I find I usually abstract away a few lines of repeated setup to setup. Also, to showcase nunit's awesome [SetUp] vs xUnit ctor use :)

I'm probably, by far, the last person that should be doing that discussion, but I find I always use a setup method.

CharliePoole commented 8 years ago

@jcansdale To the fullest extent! One of the most common errors of newcomers to NUnit is to try to use the constructor instead of [SetUp] as a "simplification."

This can, in fact, be simpler in very limited circumstances, but it's often not easy for newbies to understand the subtleties that define those circumstances. More general use would require fairly large changes in the cycle of test fixture creation and desctruction, using an approach similar to junit's.

jcansdale commented 8 years ago

To the fullest extent! One of the most common errors of newcomers to NUnit is to try to use the constructor instead of [SetUp] as a "simplification."

Okay, fair enough. I've moved from using [SetUp] to doing any construction in the "Arrange" part of my tests. I find having everything in one place less error prone.

If I was was to showcase a few of NUnit's features, they would be the fluent Assert syntax, TestCase and maybe StringAssert.

This would make the "welcome" tests something like this:

public class Tests
{
        [Test]
        public void HelloWorld()
        {
            StringAssert.Contains("World", "Hello, World!");
        }

        [TestCase("nunit")]
        [TestCase("xunit")]
        [TestCase("mstest")]
        public void TestRunner(string testRunner)
        {
            Assert.That(testRunner, Is.EqualTo("nunit"));
        }
}

Going for short, sweet and showcasing a few features in an obvious way. Thoughts?

jplebre commented 8 years ago

Agree the fluent assert should be showcased a bit more. But there's also an argument that the demo project should be kept as simple as possible.

I saw one of the builds failed, so I'll have to look at that later today. I can do any changes to the template while I'm at it :)

rprouse commented 8 years ago

Your template is almost the same as the version in the NUnit.Templates project, so I think it is fine as is.

I took a look at the one build failure and it seems to be unrelated to me. We might need to wait for the CLI team to comment on that.

CharliePoole commented 8 years ago

@jcansdale I always think of StringAssert as legacy, but it lives on because we have varied so much in how we handle strings using the fluent syntax.

On Thu, Jul 28, 2016 at 8:16 AM, Rob Prouse notifications@github.com wrote:

Your template is almost the same as the version in the NUnit.Templates project, so I think it is fine as is.

I took a look at the one build failure and it seems to be unrelated to me. We might need to wait for the CLI team to comment on that.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/nunit/dotnet-test-nunit/issues/57#issuecomment-235926725, or mute the thread https://github.com/notifications/unsubscribe-auth/ACjfhSJARYfK_-gcqiDibVna9lwYsSR0ks5qaMfFgaJpZM4JO56f .

jcansdale commented 8 years ago

I always think of StringAssert as legacy, but it lives on because we have varied so much in how we handle strings using the fluent syntax.

I've never used the fluent version, I'll have to look into that. :)

What do people think of including the following in the dotnet new template?

        [TestCase("nunit")]
        [TestCase("xunit")]
        [TestCase("mstest")]
        public void TestRunner(string testRunner)
        {
            Assert.That(testRunner, Is.EqualTo("nunit"));
        }

I find TestCase to be super useful and not terribly discoverable (or even present) in other testing frameworks.

(it's a also wink to the testRunner section in project.json)

rprouse commented 8 years ago

@jcansdale I think I would rather see us do a better job of documenting features and communicating them to the community than have a failing test in the template. I like the idea, but I also prefer to have the templates as close to ready to run as possible. Adding a test that everyone will have to delete may get stale quickly.

jplebre commented 8 years ago

edit: typed the following offline, so yeah, what @rprouse said ^^

I think I'd like a template to be "ready to use". If I have to delete as much as I'd have to type to create the template test class it'd start to be counter productive.

I think there's a point in maybe showcase the fluent assertion, but even then I think it may be too much

jcansdale commented 8 years ago

I like the idea, but I also prefer to have the templates as close to ready to run as possible. Adding a test that everyone will have to delete may get stale quickly.

I wouldn't use [SetUp] or Assert.Pass() in the majority of my tests. How about simply:

public class Tests
{
        [Test]
        public void Test()
        {
        }
}

I would rather see us do a better job of documenting features and communicating them to the community...

We could add a comment link to the most appropriate quick-start documentation. I've just had a quick search, but haven't been able to find anything obvious (for the writer of tests). There doesn't appear to be a quick-start on the wiki and https://github.com/nunit/docs/wiki/Attributes is very dense.

Looking for a pit of success. :)

FantasticFiasco commented 8 years ago

How common is it to use the AAA pattern? I use it in my tests and wouldn't cry if the test template included it.

CharliePoole commented 8 years ago

@FantasticFiasco To be clear... do you mean including the typical AAA comments that many people use? I personally avoid them, just as I don't write comments like // increment the loop index. :-)

Does this host allow selection among multiple templates? Can there be more than one?

FantasticFiasco commented 8 years ago

@CharliePoole Yes, I find them useful and it enforces that a unit test has one purpose. I've come across many test fixtures that basically only has one test that test the whole class.

What is your proposed composition of a typical test?

jplebre commented 8 years ago

No, please don't... Some people I know use it, some people are against it like the plague. Even different teams in the same company have totally opposite views on it.

The beauty of nUnit is the fact it doesn't impose on anyone a way of working, and there's nothing worse when you are imposed inheritance based tests, comment rules, or given when then frameworks.

I'd say keep it simple. Let people see a test pass from the ground up and be able to write a test straight away without extra keystrokes.

Since @CharliePoole said the use of [SetUp] should be recommended to the fullest extent - specially since xunit has a different way of doing this - I'd say keep it minimal and non-opinionated. The SetUp does not need to be deleted or edited to start working and will potentially be deleted later on in the work or used anyway.

My 2c :)

jcansdale commented 8 years ago

Let people see a test pass from the ground up...

Shouldn't we start them off with a failing test? Or am I old school now? :wink:

jplebre commented 8 years ago

Hahaha good one!! I see what you did there :) Yes the TDD say failing rest first, but knowing the framework is working, picking up tests and runs them correctly is a good starting point.

Knowing framework is working will allow you to change the test and "know" with confidence it's failing for the "right reasons" (picking up on TDD lingo there) - the right reasons being you wrote a failing test and now need to make it green :)

CharliePoole commented 8 years ago

Actually, as a tdd guy, I want them to make it fail, which requires adding code.

jplebre commented 8 years ago

'Assert.that(true, is.equalto(false));'

I think I'll step back now to let other people voice opinions. I think the argument of keeping it simple and ready to write code is a strong one, and it should show that nunit + test runners are working as expected, but I really don't have the experience to argue it any further.

Even as a TDD person I would immediately write a failing test in my applications context, with full confidence it would pass when I write the minimum amount of code to make it pass.

I'll see what the feeling is by the time the CLi team gets back to us. Let me know as well. Changing it is no biggie :)

FantasticFiasco commented 8 years ago

@jplebre You are probably right. Keep the template as close to the metal as possible. I've already created ReSharper templates for test stubs they way I like to write them, and can continue to use them since the template won't get in the way.

jplebre commented 8 years ago

hmm, the test for the project is failing on a few of the OS. Seems there's a stream already in use. Would this be an nunit issue?

rprouse commented 8 years ago

Fixed by dotnet/cli#3960. Thanks @jplebre :tada:

halex2005 commented 7 years ago

dotnet new CLI now uses the new templating engine. Unfortunately, this project template was forgotten by MS guys. Fortunately, it wasn't hard to write test project template for new engine. I've added it to common list of available templates too. Hope this helps anyone, PRs are appreciated.

jnm2 commented 7 years ago

That's really cool!