Distinguishing between failed assertions

[vivin]

I previously submitted a commit where the log callback also provided information about the assertion that failed. Perhaps the way I went about implementing wasn't the best and could warrant more discussion. My rationale was as follows:

• The assertion-type provides information about the type of assertion that was violated. This is pretty useful when doing automated tests with PhantomJS and QUnit.
• Currently the throws assertion has an expected value of null. This is because the expected value can be a regular expression. When reporting the results of tests, we can tell whether the test failed or not by examining the result property from log. However, the decision to show expected and actual values (which don't apply in the case of throws when using a regular expression) cannot be safely made by simply examining the expected value and checking if it is null. There can be cases where null is an actual expected-value. Instead, it would be safer and more obvious if information was provided about the actual assertion that failed, and then decide whether to show the expected and actual values.
• This problem also exists in the current QUnit reporting When a throws assertion that expects an exception fails (and it uses a regular expression), the expected value is shown as null, which is not correct. In this case, if we had information about the type of assertion that failed, we could examine that and decide to not show the expected and actual values if the assertion was a throws assertion.

Perhaps we don't need something like an AssertionType public property for QUnit. We could simply provide the name of the test method that failed and that should be sufficient. The reason I originally used an AssertionType public property was so that it would be easily maintainable; we could deprecate the name of a test method and not have to go through and search for all usages of that name; we could simply simply change the method name for AssertionType. Since we are using that and not raw strings, the change would be effected everywhere. Perhaps this property can be an internal/private property to QUnit.

Jörn suggested a workaround where you could simply add that information to the message returned by throws. While it works, I think the solution is slightly ad-hoc and not as robust as an actual property that provides information about the assertion that failed.

Thoughts?

[vivin]

I've got three branches that detail three approaches. Approach #1 (99ea1be) simply uses raw strings; this is probably the easiest to make and doesn't really create anything additional. Approaches #2 (ee233a7) and #3 (0508b89) are similar to my original commit, expect that #2 doesn't expose the AssertionType object. I'm leaning towards #1 myself since we already hardcode the names of the methods in a few places. #2 and #3 would introduce inconsistencies. Comments are appreciated! I honestly feel that adding the assertion that failed, to the information returned by the log callback, would be useful.

[vivin]

No feedback? :(

[jzaefferer]

I still don't want to add assertion types, but based on the description here I found a problem worth addressing directly. throws will now provide useful output for its expected value, in the regexp and constructor case, otherwise it'll still output null. If your actual value is an instance of the built-in Error object, it'll output that as something useful as well.

[vivin]

May I ask why you are reluctant to add assertion types? :) I can't see a downside to it although I may be missing something. I know that unit-testing frameworks in Java like TestNG report the failed assertion-type so I do think that it's useful and I don't really see any harm.

[jzaefferer]

The harm is in the additional complexity that any code additions brings with it, both in the code itself and in documentation. I'm not generally against additions, but being skeptic helps to keep things sane. Your original description has "pretty useful" as the only remaining argument, as the throws issues were addressed (right?). I'm looking for a valid usecase, both to convince me of the additions value and to document it.

[vivin]

I completely understand your skepticism as far as adding code and complexity. The complexity in this regard is rather low, I believe (just the addition of a single property to the failure).

The throws issue seems to be addressed for the most part. I should have elaborated on the "pretty useful" part: at work I've integrated QUnit with TestNG so as to have CI for Javascript. We'd like to keep track of the type of the different assertions that fail (similar to what we do for TestNG); this gives us some insight as to the most common types of failures that our code encounters. Currently I'm doing this by patching qunit.js so that the failures report the kind of assertion that failed as well.

I understand if this is not enough to convince you - I can make do by patching it to include that information on failure :). I just think that including the type of assertion that failed can be pretty useful for people who wish to keep track of the kinds of assertions that fail (for metrics/statistics), it also seems like an obvious piece of information to have IMHO.

[Krinkle]

@vivin Just out of curiosity, how is the type of assertion (as opposed to the name/description of the assertion) relevant?

Does it statistically matter whether something does !!foo === true, foo instanceof Bar or foo === bar? Also, plugins can extend the assert object:

QUnit.assert.lt = function (actual, expected, message) {
    QUnit.push( actual < expected, .. );
};

[vivin]

@Krinkle It's just something that we keep track of. For example, to see the types of failures that occur after a specific change. May not be very statistically significant.

Good point about the plugins. I didn't know about those. I can see how they would have to modify their code to pass back the name of the assertion.

Like I said, this is not a deal-breaker. It was just something I thought would be useful.

[Krinkle]

@vivin I'm just trying to understand why anyone would want to know what method the assertion used to get its result.

It isn't significant for anything. Is there some odd priority that you assign internally to failures from assertTrue vs. assertEquals? Both can be a major problem, what does it matter?