I think such situations are OK, as long as you understand why the test failed before and verified that the new test really tests the same thing.
Aaron Meurer On Tue, Sep 2, 2014 at 9:26 PM, Chris Smith <[email protected]> wrote: > At https://github.com/sympy/sympy/pull/7945 discussion involving a change > that I made to a test that was added in response to issue 6533 has arisen. > In that issue a particular expression (which is slow to compute) raised an > assertion error, identifying that a change needed to be made in the code. > The code was fixed and that original slow expression and 2 others were added > as tests. > > I noticed this issue while waiting for a local test to run. Specifically, I > noticed that the slow test was taking about 2 minutes. I went back in > history to where the change was made, backed up to the previous commit, and > experimented with simpler expressions (similar to the one identified in the > issue) that gave the same error. I replaced the original tests with that > expression. > > The question that has come up is whether the original error-producing > expression needs to appear in the test suite. I think not and offer the > following guide for adding tests for code modified after a problem has been > identified: > > 1) use the original failing expression unless a much simpler expression will > exhibit the same failure or a similar expression will run much faster; if > the original expression runs quickly and is not too complicated, it is not > necessary to spend much time trying to find something simpler. > 2) add any additional tests needed to make sure that all the code added is > covered. > > The reason for not using the original expression is that in debugging a > problem one often finds what the key failing issue is and can pinpoint that > portion of code with a simpler test. > > I just happened across this issue and went back to the commit before the > commit that fixed the issue (0fa7b9730e56759e5f4bc3752143f5) and found a > simpler expression that failed in the same way as the original expression > and covered the few lines that were added in the fix. On my computer it cuts > about 2 minutes off the suite test time. I would like to make this change in > PR https://github.com/sympy/sympy/pull/7945 . It has been requested that > somebody else confirm or reject my proposal. > > -- > You received this message because you are subscribed to the Google Groups > "sympy" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To post to this group, send email to [email protected]. > Visit this group at http://groups.google.com/group/sympy. > To view this discussion on the web visit > https://groups.google.com/d/msgid/sympy/b9d1dc4d-6f16-4b69-ae74-edc14f18a2f3%40googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "sympy" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at http://groups.google.com/group/sympy. To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CAKgW%3D6K4-uA%3DuMY%3DAypiGFk1Dj2nEp2Vk0YAt3MT5X5wd%3Dmg0A%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
