+972 52-548-6969
Got to debug is a bug, fix your error messages
time to read 1 min | 144 words
This is part of the RavenDB test suite.
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:
Means that I get the actual error message right there and then, no need to do anything special.
Comments
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)
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.
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)
Correction for above: Assert.IsNull should be used
Assert.False(true,..) is a really confusing way to write an assertion.
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.
He needs to show the actual error message in the tests.
I hate Assert.False(true.... I was wondering if that was some typo.
And also why you once break out of while (and method) and in other place you return?
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.
Whoops, IsNull(message) should be IsNull(message, message).
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.
It will throw an exception if x.Severity is null.
Gian,
Assert.IsNull(message, message) would do this (the second parameter being the failed assertion message).
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