1

Background: I'm asking this question as a UI test engineer. We have a small software team using TFVC on a TFS repository. We use the mantis bug tracking system to follow up on issues.

Here's the situation: While manually testing a program, I found a bug and reported it to the devs. A few days later, a dev has let me know the issue was resolved. Some other day, I started testing to see if the bug was resolved but found the exact same bug.

problem: Someone made a change in the code in order to fix the bug. A few days later, the code got changed again, reintroducing the bug into the program. I don't know if this occurred because of the auto-merge feature in TFVC or that the change was made by another programmer who did not know about that fix.

question: Are there ways to prevent such errors from ever happening? Or is this just sloppy work from that dev who changed the code back to its original form?

  • This is an error from your perspective because you didn't like the results, but TFS can not make that value judgement. What if the fix actually made it worse and you needed to go back to the original code? How can TFS tell the difference between the two scenarios? – cdkMoose Sep 10 at 17:55
  • You're right about TFS/TFVC just doing what it is programmed to do. In that case, I tend to think that the real error is actually the way TFS handles such cases, no? – PixelPlex Sep 11 at 9:01
  • Possibly, but at the end of the day, TFS has provided a set of rules defining behavior and while you can disagree whether the rules are appropriate, I would say TFS followed the rules and it is up to the users to understand those rules. – cdkMoose Sep 11 at 12:58
4

There are two techniques that work together to avoid this type of scenario from occurring:

Step 1: write a failing test before fixing the bug

When the bug has been found, the first thing to do is to write a test that will pass when the bug is fixed and thus it fails whilst the bug still exists. The dev then fixes the bug and in the process makes the test pass.

That way, when the second dev undoes the fix, a test will fail, alerting them to the fact that they are (re)introducing a bug.

Step 2: use pull requests

Rather than committing changes directly to TFS, use pull requests instead. That way, each change is checked by someone other than the developer making the change. This effectively ensures all changes are code reviewed. That not only guards against issues such as described in this question, but provides the many other benefits around code quality associated with code reviews.

  • By "TFS repository" I think the OP means using Team Foundation Version Control instead of Git. You can submit a code review in TFVC though. – Greg Burghardt Sep 10 at 11:21
  • @GregBurghardt, good point. To use pull requests, Git, rather than TFVC would have to be used for the project in TFS. But Git, rather than TFVC should be used anyway as it offers way more benefits than just pull requests. No more idiotic change conflicts when you are the only developer working on a project for example, which is the worst of many negative features of TFVC. – David Arno Sep 10 at 11:39
  • 2
    Oh, and don't forget auto-merges that blow away critical changes that should have triggered merge conflicts. In fact, this might be the issue the OP is running in to. – Greg Burghardt Sep 10 at 11:48
  • Updated my question for clarification. – PixelPlex Sep 10 at 12:30
  • @GregBurghardt you have a valid point. Now I think about it, we have no idea if the issue was caused by TFVC or another team member. – PixelPlex Sep 10 at 12:30
2

I wholeheartedly second David Arno's suggestions about failing tests and code reviews. Also, there are often ways to write the code to make it less susceptible to that kind of error in the future.

For example, if the error was created by calling a function before its prerequisites were met, you can split your classes differently so that would create a compiler error. If you keep forgetting to handle a special case, you can move the special handling to one central place. If a variable is sometimes used uninitialized, you can change your code so the variable isn't created until there is a valid value to put into it. If it's not clear what the purpose of some inline validation code is, move it to a well-named function. If you can't think of a way to make your intention clear in the code, at the very least, add a comment that explains why you made a change.

The idea is to make it clear as a bell why your fix is in there, so the next person coming along doesn't think it's a mistake. This is especially important if your fix accidentally introduces another bug. If your code looks like a mistake or a coincidence, the next dev will just undo it. If it's easy to discern the intention of your code, the next dev will fix the bug you accidentally introduced without reintroducing the one you fixed.

  • Those are good guidelines indeed, but unfortunate, it doesn't prevent the devs to be sloppy about it, if it is not enforced in some way. – PixelPlex Sep 10 at 13:42
  • You definitely need the tests and code reviews. I'm mostly pointing out what you might want to look for in the code review. – Karl Bielefeldt Sep 10 at 16:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.