Treat exceptions as equivalent while performing diffbehavior

[Abhiram98]

While running diffbehavior on the below functions, crosshair reports a counter-example (because they raise different exceptions).

def original(int_list):
    count = 0
    for i in int_list:
       count +=i
    return count

def rewrite(int_list):
    count = 0
    for i in range(len(int_list)):
        count += int_list[i]
    return count

Crosshair output:

Given: (int_list=0),
  temp.test.original : raises TypeError("'int' object is not iterable")
   temp.test.rewrite : raises TypeError("object of type 'int' has no len()")

It would be nice to have a CLI argument where exceptions are treated as equivalent, and other counter-examples are raised (if any).

[Abhiram98]

I would be happy to contribute towards a patch here, if you think this is a valuable feature

[pschanely]

Interesting! Do you think a change in exception type should still be reported in this mode? Or perhaps we just add a expection_equivalence= options with values for "all", "same_type" , or "type_and_message". Realistic examples would be nice here to motivate the decision.

If we come to an agreement about the above, I'd love a contribution! The core bit of code we care about here is where we repr the exception in the describe_behavior function in diff_behavior.py. We'd have to add the new option in main.py and pass it through various functions to get it where it needs to go. (you'll notice attractive-looking AnalysisOptions arguments, but please don't add this option to that - that's just for global options, and our option should be specific to the diff behavior command) I'd want to see a test or two in diff_behavior_test.py as well. Happy to help you along the way too, either here or in gitter.

[Abhiram98]

I like the idea to add an option expection_equivalence= (with selections for "all", "same_type" , or "type_and_message"), because I can see how spotting differences based of exceptions would be valuable. However, I couldn't find a case where two code blocks produced different exception types for the same input.

What do you think? What should the default value for the option be? I think the value same_type would be nice here.

[pschanely]

I like the idea to add an option expection_equivalence= (with selections for "all", "same_type" , or "type_and_message"), because I can see how spotting differences based of exceptions would be valuable. However, I couldn't find a case where two code blocks produced different exception types for the same input.

One possibly common reason for different exceptions happens when you edit your logic that creates an exception message, but introduce a bug, and now your intended exception becomes some other exception. Most people would want to know about this. It's also the kind of change that is less likely to have unit test coverage.

What do you think? What should the default value for the option be? I think the value same_type would be nice here.

I agree that same_type seems attractive. That said, I'm biased towards keeping the default doing what it does today, which is a vote for type_and_message. That setting is also the right option for strictly checking refactors - people using diff_behavior in this case are likely trying to get as close as possible to a "guarantee" that their change is not user-visible in any way. Your example above is actually pretty interesting; most people would accept this kind of behavior change, but some portion of those would probably still want CrossHair to tell them that it's happening. I think I'd go for type_and_message for now; though I am happy to have my mind changed with sufficient evidence that the setting is too noisy for most people.

[pschanely]

This shipped in v0.0.80.
Thanks again @Abhiram98!