Parameterized method being called with no parameter

[mbenningfield1]

[TestCase, TestCaseSource(typeof(BooleanTestData))]
public void Test(bool authorized)
{
  ...
}

public class BooleanTestData : IEnumerable
{
  public IEnumerator GetEnumerator()
  {
    yield return true;
    yield return false;
  }
}

-OR-

[TestCase, TestCaseSource(typeof(BooleanTestData), "Value")]
public void Test(bool authorized)
{
  ...
}

public class BooleanTestData
{
  public static IEnumerable Value
  {
    get
    {
      yield return new TestCaseData(true);
      yield return new TestCaseData(false);
    }
  }
}

 -or-

public class BooleanTestData
{
  public static IEnumerable Value
  {
    get
    {
      yield return true;
      yield return false;
    }
  }
}

 -or-

public class BooleanTestData
{
  public static bool[] Value
  {
    get
    {
      return new bool[] { true, false };
    }
  }
}

results in this:

<test-case id="0-1001" name="Test(True)" fullname="MyDll.TestAuthorizer.Test(True)" methodname="Test" ...
  <reason>
    <message><![CDATA[OneTimeSetUp: true]]></message>
  </reason>
</test-case>
<test-case id="0-1002" name="Test(False)" fullname="MyDll.TestAuthorizer.Test(False)" methodname="Test" ...
  <reason>
    <message><![CDATA[OneTimeSetUp: true]]></message>
  </reason>
</test-case>
<test-case id="0-1003" name="Test" fullname="MyDll.TestAuthorizer.Test" methodname="Test" ...
  <properties>
    <property name="_SKIPREASON" value="System.Reflection.TargetParameterCountException : Incorrect number of parameters specified for TestCase" />
    <property name="_PROVIDERSTACKTRACE" value="   at NUnit.Framework.TestCaseAttribute.GetParametersForTestCase(IMethodInfo method)" />
  </properties>
  <reason>
    <message><![CDATA[OneTimeSetUp: true]]></message>
  </reason>
</test-case>

It appears that an extra call to the parameterized method is being generated that uses no parameters.

[appel1]

It's because you've specified an empty TestCase in addition to creating tests using TestCaseSource.

[rprouse]

To expand/clarify what @appel1 is saying, the [TestCase] is unneeded. It will try to create the third test and fail because the parameters don't match.

[mbenningfield1]

OK, thanks. I wish I had some free time, because the docs really need a lot of work. I will check back when my current side project is finished; maybe I'll have some more time then.

[CharliePoole]

@mbenningfield1 Hope you do have some time, but meanwhile could you point to where they are misleading regarding TestCase?

[mbenningfield1]

Not misleading; perhaps more counter-intuitive; and with TestCaseSource,
not TestCase.

I realize that the docs mention that TestCaseSource additionally
identifies a method as a test case, but
it is a mention in passing.

The name of TestCaseSource suggests -- grammatically speaking -- that is
primarily a 'source', not a 'testcase', and
would therefore intuitively be used in conjunction with a TestCase
attribute.

Now, this is not my first trip to town; I understand that there is no
way in Hell to change such things. My point is that
the documentation (and not just for NUnit; most declarative frameworks
have this problem: SQL, git, etc) is written,
as it usually is, by those that have an intimate understanding of the
product, and so can't see it from the outside.

Once you know the paths, you no longer see the maze. Right now the docs
are a pretty good technical reference,
but they are not yet a very good manual. What's needed is not a a few
tweaks or even a rewrite; it is a redirection
into a separate or companion document set.

Of course, that sort of thing is usually at the very bottom of the TODO
list, because nobody wants to do it. I don't
particularly want to do it, but I would; the problem is that I'm at
least 18 months out on the current side project I'm
working on. Just keep in mind that someone's going to have to do it
eventually.

On 10/30/2016 1:05 PM, CharliePoole wrote:

@mbenningfield1 https://github.com/mbenningfield1 Hope you do have
some time, but meanwhile could you point to where they are misleading
regarding TestCase?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1872 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AVe-Xq7MflDjy49i9CbBg8NHOYXitXt0ks5q5NyFgaJpZM4KkUQs.

[CharliePoole]

Well, the good news is that I've only seen this particular thing come up once before. 😄 However, I understand that it seems like a pretty natural thing to do.

Maybe it's a bug that we allow you to use TestCase without an argument and then tell you it's wrong!

[mbenningfield1]

As I understand it (granted, I'm a noob with NUnit), TestCase with no
args is exactly the same as Test.

Any time you have 2 pieces of an API that do exactly the same thing, its
a flag: one of them is either wrong

or not needed, and pieces that aren't needed are "wigglies in the soup".

I would say that if it's at all possible, amend the API to require args
on TestCase.

On 10/30/2016 10:09 PM, CharliePoole wrote:

Well, the good news is that I've only seen this particular thing come
up once before. 😄 However, I understand that it seems like a pretty
natural thing to do.

Maybe it's a bug that we allow you to use TestCase without an argument
and then tell you it's wrong!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1872 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AVe-XpPjvKbCmoiQqBnLE8cK47Az99Hnks5q5Vv1gaJpZM4KkUQs.

[CharliePoole]

It was never intended to be so, but the implementation allows it to be used that way. A few people have taken it that way, tried it out and found out that it works. That, of course, explains why it's not covered in the docs as much as anything else.

[CharliePoole]

Reopening this for discussion: Should we make it impossible to use TestCaseAttribute without args?

[rprouse]

As I quick test, I tried to add a private default constructor to TestCaseAttribute to hide the params constructor, but the params constructor still gets called. Therefore I don't think we can disable it at compile time.

We could switch the null check in the params constructor to throw which would give a more meaningful error. It is still a runtime bug, but I would be okay with that since it is giving the user more information.

That said, we also allow a [Test] with several TestCases or with a TestCaseSource. It doesn't cause side effects, but should it be an error? My feeling probably not.

So, my vote is 👍 but I don't have strong feelings.

[CharliePoole]

In the past, I've used a constructor like `TestCase(object arg1, params object[] remaining) to force the inclusion of a first argument. It makes the constructor code a bit more complicated but doesn't impact the user.

Extra [Test] and [TestFixture] attributes are a legacy thing that we allow for backward compatibility. It takes a small amount of special code for each of them - more for [TestFixture] because of inheritance. For example, we only generate a test for [Test] if it's the only attribute present on the method that generates tests. Just one annoying extra check. 😄 We could do the same for [TestCase] but I'm more inclined to make it not compile.

[rprouse]

That is a better idea for making it not compile, I didn't think of that. Base on that, I am more in favour of the idea 👍

[CharliePoole]

Changed the labels but leaving it here in discussion for a beat. Anybody else have thoughts on this?

[mbenningfield1]

For my part, I'm a firm believer in leaving client code as little room
as possible for making missteps.

On 10/31/2016 6:41 PM, CharliePoole wrote:

Changed the labels but leaving it here in discussion for a beat.
Anybody else have thoughts on this?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1872 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AVe-XqB1JxIgk9_l9tv_BZjUHxEx3MLyks5q5nykgaJpZM4KkUQs.

[CharliePoole]

Five days... anyone not commenting missed their chance. 😄

Adding to backlog.

[CharliePoole]

@rprouse My idea for making it not compile works... but doesn't. ☹️

That is, it does prevent [TestCase()] from compiling, but it turns out we need it for at least two reasons:

1. We use it for methods without arguments that have a return value.
2. We use it for methods with all the arguments optional.

So the error will have to be at run-time rather than compile-time.

[rprouse]

Runtime is less than ideal, but a better error message will be an improvement.