Skip to content
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

Add Assert.Equal(expected, actual, message) overload #350

Closed
hpshelton opened this issue Mar 11, 2015 · 17 comments
Closed

Add Assert.Equal(expected, actual, message) overload #350

hpshelton opened this issue Mar 11, 2015 · 17 comments

Comments

@hpshelton
Copy link

I'd love to see feature parity with MSUnit and NUnit, which both already support overloads for equality with user-specified messages. I believe a new overload in EqualException would be required:

public EqualException(object expected, object actual, string message)
     : base(expected, actual, message)

As would new overloads in EqualityAsserts.cs:

public static void Equal<T>(T expected, T actual, string message)
public static void Equal<T>(T expected, T actual, IEqualityComparer<T> comparer, string message)

But as far as I can tell, that's all the changes that would be required. Are there additional dependencies I don't see at first glance or a design reason these overloads aren't already available?

@bradwilson
Copy link
Member

We make vague mention of it here: http://bradwilson.typepad.com/blog/2008/03/xunitnet-10-rc2.html

We obsolesced most of the Assert methods which take user messages. The only ones we left are those on Assert.True and Assert.False, which tend to be catch-all asserts which might require documentation. We will be removing the obsolesced methods in 1.0 RTM, so please move your calls to the message-less variants.

I could not find a blog post that talked about "why", even though we've mentioned it several times. I'm just not sure it every got a permalink.

We are a believer in self-documenting code; that includes your assertions. If you cannot read the assertion and understand what you're asserting and why, then the code needs to be made clearer. Assertions with messages are like giving up on clear code in favor of comments, and with all the requisite danger: if you change the assert but not the message, then it leads you astray.

To support people writing better assertions, v2 includes a NuGet package that ships the assertion library in source code. The Assert class is a partial, so you can add whatever assertions you like to the built-in set. We've even gone so far as to publish gists with extra assertions, just to show people how it's done: https://gist.github.com/bradwilson/7797444

@feO2x
Copy link

feO2x commented Mar 11, 2015

If you really want to have messages you could add Fluent Assertions or maybe xbehave to your test projects and use their syntax. Fluent Assertions even throws xunit.net exceptions if it encounters its presence.

@bradwilson
Copy link
Member

The move to make our assertions available as source was also motivated by a desire to make them optional. We've heard from a decent portion of our user base that they end up using other assertion libraries like Shouldly or Fluent.

@hpshelton
Copy link
Author

Thanks, all. I'm working with corefx and missing the overloads, but I'll talk to some people about possibly creating custom equality assertions in that project.

@bradwilson
Copy link
Member

The easiest porting path would be to use the source NuGet package and just write it yourself. :)

rainersigwald added a commit to rainersigwald/codeformatter that referenced this issue Aug 20, 2015
xunit does not support a "message" field in its asserts.  This is
intentional: xunit/xunit#350.

MSBuild has used the message field, though, and it seems wasteful to just
that information away.  This introduces a new converter that extracts the
message (if the extra argument in an assert is a string literal) into a
comment.
rainersigwald added a commit to rainersigwald/codeformatter that referenced this issue Sep 1, 2015
xunit does not support a "message" field in its asserts.  This is
intentional: xunit/xunit#350.

MSBuild has used the message field, though, and it seems wasteful to just
that information away.  This introduces a new converter that extracts the
message (if the extra argument in an assert is a string literal) into a
comment.
@ghost
Copy link

ghost commented Feb 19, 2016

@bradwilson I think it is a mistake to remove user messages. A good reason for adding a user message is for adding information that might be useful to track down the error. For instance if you are writing a theory with memberdata passed to the test data, it might be useful to display some information derived from that memberdata to the assert failure so it is easy to see what exact context the assert failure happens in.

Because of the lack of user messages, I have now many tests where I would like to use Assert.Equals but I am using Assert.True instead (where I can specify a user message).

@bradwilson
Copy link
Member

@bluemmc We won't be changing our minds on this issue.

The assertion library is optional in 2.x, so if you don't like our assertions, you can remove the xunit.assert NuGet package, and use one of the plethora of third party assertion libraries. Or, you can bring in our assertion library via source instead of binaries (xunit.assert.source) and make whatever modifications you'd like, to create your own assertion library.

@clairernovotny
Copy link
Member

Among others, FluentAssertions works quite well with xUnit.

@DLemberger
Copy link

I have an easy workaround for this, as the Assert.equal function works with Strings you can easily add the Message within this String.

As a little example, where i use it myself:
At the loginpage we check for valid and invalid passwords
instead of Assert.Equal(true,password.CheckValid());
you can make the Assert.Equal("The password is: valid", "The password is: " + password.CheckValid()); with a return value of a String valid/invalid
So if whatever you want to Test matches it doesn't bother you and if not you will get a line like Assert expected: The password is: valid, actual: The password is: invalid

@ssg
Copy link

ssg commented Mar 21, 2016

Messages were useful to provide debugging information (test state), to identify the failure. Installing a separate library and to spend time to learn it, deal with its own set of problems etc to have that functionality is a quite a big overhead. I'm currently resorting to Debug.WriteLine()'s and not liking it.

@ghost
Copy link

ghost commented Mar 22, 2016

Debug.WriteLine don't work as they are ignored by xunit and their proposed alternative is ignored by visual studio.

It appear XUnit is trying it's best to make it impossible to get any information out of unit tests and their developers are taking an extreme view, trying their utmost to ignore any sensible user feedback on the subject (of asserts, writeline etc).

@feO2x
Copy link

feO2x commented Mar 22, 2016

I have to disagree with @ssg and @bluemmc - assertion libraries like FluentAssertions are usually very easy to learn (you only need a few minutes in my opinion) and they provide a lot of flexibility for custom assertion messages. If you just want to output some additional test state (e.g. performance related data), then use xunit's ITestOutputHelper or some more advanced mechanism: https://xunit.github.io/docs/capturing-output.html (works in R# runner, VS Test Runner, and console runner for me).

@ssg
Copy link

ssg commented Mar 22, 2016

The thing is: xUnit.Net's team's rationale to remove the feature was "the code itself should be sufficient to explain why the test failed" but the framework does not provide me any scaffolding to provide additional state of the test, only the input itself. The input isn't necessarily the only part of the test state. That's a problem with debugging iterative tests, or tests that have to calculate the input first.

"Data-driven" tests could be used in some of those cases. To identify the failing row, you have to assign sequence numbers to rows one by one, or implement a whole new IEnumerable class from scratch. It's just too much where a simple , "failed at iteration #" + i) addition would work fine. It's well-known, universal and simple. Incorporating new third party libraries, learning "some easy ad-hoc stuff", re-implementing your tests, ITestOuputHelper's etc they all are too much frictions to me so I resort to ugly tricks. Was that xUnit.net team's intent? I guess not.

If xUnit team wants to eliminate the use case of Assert.Equal(2, number, "the number is not 2"); they should at least allow Assert.Equal(2, number, state: new { seed = 123 }) kind of variant.

@bradwilson
Copy link
Member

You can provide messages to Assert.True and .False. If you simply cannot live without messages (and refuse to use a different assertion), you could always fall back to:

Assert.True(number == 2, "This is my message");

@bradwilson
Copy link
Member

BTW, our rule here for assertion messages is not new, and it's nothing something we "removed"; we've never had this feature in the 8½ years that xUnit.net has existed.

@ssg
Copy link

ssg commented Mar 22, 2016

@bradwilson if I use Assert.True I lose code semantics and good amount of info on test output. Wasn't the whole point of removing the message is to make code more meaningful? The workaround contradicts with the intent.

I was giving xUnit a shot for adoption so "it's been always like this" doesn't really work for me. Thanks.

@bradwilson
Copy link
Member

This conversation has devolved to the point where locking it is the right answer.

@xunit xunit locked and limited conversation to collaborators Mar 22, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants