Skip to content

TestCaseAttribute: ExpectedResult should support same value conversion as normal method arguments #2734

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

Closed
BrightLight opened this issue Mar 7, 2018 · 6 comments

Comments

@BrightLight
Copy link
Contributor

BrightLight commented Mar 7, 2018

The TestCaseAttribute has support for some argument types that can't normally be used in attributes, like DateTime. For instance, a date can be given as a string which will then be converted into a DateTime, like this

[TestCase("2017-06-10")]
public void ReturnGivenDateTime(DateTime inputValue)
{
  Assert.That(inputValue, Is.EqualTo(DateTime.Parse("2017-06-10")));
}

"inputValue" will have the correct date.
Sadly, it doesn't work the same way for "ExpectedResult", the return value of the method.
I would like to be able to do this:

[TestCase("2017-06-10", ExpectedResult = "2017-06-10")]
public DateTime ReturnGivenDateTimeWithReturnValue(DateTime inputValue)
{
  return inputValue;
}

but I'm getting this as a result:

Expected: "2017-06-10"
But was:  2017-06-10 00:00:00

The return value (of type DateTime) is compared to the ExpectedResult (of type string).

I can provide a pull request, but after reading your contribution guidelines I figured I should suggest this first as a request.

@jnm2
Copy link
Contributor

jnm2 commented Mar 7, 2018

Good find. This seems like it could be connected to the work on #2104.

@StanEgo, do you want to try to handle this as well or leave it for afterwards?

@BrightLight
Copy link
Contributor Author

BrightLight commented Mar 7, 2018

This would be my suggested fix: BrightLight/nunit@934a2a5
(my goal was to change as little as necessary, therefore ExpectedResult as I didn't want to change ITestCaseData)

@jnm2
Copy link
Contributor

jnm2 commented Mar 7, 2018

I'm open to that approach. @nunit/framework-team?

@CharliePoole
Copy link
Member

@jnm2 #2104 is already pretty complicated.

@BrightLight Can you do a PR?

@CharliePoole
Copy link
Member

@BrightLight I think we all believe this should be fixed. As to the implementation, it's easier to review that in a PR.

One question comes up, however. Is it sufficient to convert the expected value or do we have to wait to get the actual return?

@BrightLight
Copy link
Contributor Author

Okay, created pull request #2735 for that.

As to the question whether it is sufficient to convert the expected value, I think converting the value up-front should be enough (though I might not know every supported scenario of course). If the ExpectedResult cannot be converted, I think it should fail fast, not first run the test.
We might have a more precise target type when the actual return value is known but converting into the specified type should suffice in my opinion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants