Closing an Issue

smichr edited this page Oct 23, 2014 · 1 revision
Clone this wiki locally

An issue identifies some expression that

  • gives a wrong result
  • raises an error (other than NotImplemented)

An issue can be closed when the issue no longer causes the problem. It is customary to add and refer to the failing expression in the test suite with a citation like

# issue 1234
assert something_involving_the_failing_expression

There are different views on whether the exact expression that identified the failure needs to be included or not. Generally, the failing expression should be included unless one of the following applies:

  1. a much simpler expression that exhibits the same failure; this is especially true with it is just a pair of arguments (out of many) that raises the issue or even a single argument that raises the issue.
  2. a similar expression that runs much faster can be easily found.

Remember: there are a myriad of ways to be wrong and we do not need to test all of them, just the salient feature --if it can be identified easily -- that raises the issue. e.g. maybe factorint(very_big_prime)/0 returned oo instead of zoo. One should obviously prefer to test that 1/0 returns zoo.

An argument for always including the original expression is that if the same expression fails in the future -- for a different reason -- it feels like a regression, we must "fix that the code to handle that expression again!" If such an expression is fast to compute there is not much harm done in adding the expression, preferably as part of the tests that are added when the code is being fixed. A special time that this argument becomes more grey is when one finds an issue that was fixed by some code in the past but was not identified as being related at that time. Should the test be added? Quoting from a mailing list discussion

some people advocate eliminating unit tests if they have been exercised so thoroughly that they're unlikely to ever trigger again: their information value has decreased, and they hamper architectural changes because they tend to need adaptation

So it might be the case that the expression represents something that is really a non-issue. But if it's a fast test then spending time researching what fixed it, what the underlying problem is, etc... may be a waste of time. Just add the test, close the issue, and move along.