Ayende @ Rahien

Hi!
My name is Ayende Rahien
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

@

Posts: 5,947 | Comments: 44,541

filter by tags archive

Got to debug is a bug, fix your error messages


This is part of the RavenDB test suite.

image

It has a bug.

Can you see it?

One of the things that I learned when working on the Castle project is that errors are important. In fact, they are really important, so it is worth the time to check them very carefully.

In this case, this code would fail but won’t tell me why it is failing.

Changing it to this:

image

Means that I get the actual error message right there and then, no need to do anything special.


Comments

Scooletz

Any with predicate will return true if the backupStatus.Messages is empty, so the Assert.False will throw. One of the solutions is what you provided, the second doing .Any() && .Any(yourPredicate)

Stefan

What about all other messages with severity error other than the first? They still get dropped.

Why not use Assert.Fail(message.Message); ? I find Assert.False(true,...) confusing.

Pop Catalin

This is a bit of funny code: if (message != null) Assert.False(true, message.Message);

expecially the 'Assert.False(true...' part.

Can easily be replaced with:

Assert.Fail(message.Message) or Assert.IsNotNull(message, message.Message)

Pop Catalin

Correction for above: Assert.IsNull should be used

Ryan

Assert.False(true,..) is a really confusing way to write an assertion.

Josh Kodroff

Umm... what's wrong with:

Assert.That(() => backup.Messages.Any(x => x.Severity == BackupStatus.BackupMessageSeverity.Error), Is.True, "Expected to find a backup message error, but didn't find one.");

Pretty explicit in my mind.

João P. Bragança

He needs to show the actual error message in the tests.

Karep

I hate Assert.False(true.... I was wondering if that was some typo.

Karep

And also why you once break out of while (and method) and in other place you return?

Matthew Wills

I'd likely lean towards:

var message = backupStatus.Messages.Where(x => x.Severity == BackupStatus.BackupMessageSeverity.Error).Select(z => z.Message).FirstOrDefault();

Assert.IsNull(message);

to avoid the if.

Matthew Wills

Whoops, IsNull(message) should be IsNull(message, message).

Gian Maria

When a unit test fails the name of the unit test is important, but if you have some more descriptive failure message it will help you finding the real cause of the test failure.

I agree with matthew that an Assert.IsNull is a clearer form of assertion, but the key point is including the real error message in the unit test result to have a quick hint of why the test is failing.

Lookman

It will throw an exception if x.Severity is null.

Matthew Wills

Gian,

I agree with matthew that an Assert.IsNull is a clearer form of assertion, but the key point is including the real error message in the unit test result to have a quick hint of why the test is failing.

Assert.IsNull(message, message) would do this (the second parameter being the failed assertion message).

It will throw an exception if x.Severity is null.

I don't think any of the proposed solutions will do that. They may have issues if backupStatus.Messages was null, or contained a null entry, though.

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

No future posts left, oh my!

RECENT SERIES

  1. RavenDB Sharding (3):
    22 May 2015 - Adding a new shard to an existing cluster, splitting the shard
  2. The RavenDB Comic Strip (2):
    20 May 2015 - Part II – a team in trouble!
  3. Challenge (45):
    28 Apr 2015 - What is the meaning of this change?
  4. Interview question (2):
    30 Mar 2015 - fix the index
  5. Excerpts from the RavenDB Performance team report (20):
    20 Feb 2015 - Optimizing Compare – The circle of life (a post-mortem)
View all series

RECENT COMMENTS

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats