How can my team avoid frequent errors after refactoring?

Question

To give you a little background: I work for a company with roughly twelve Ruby on Rails developers (+/- interns). Remote work is common. Our product is made out of two parts: a rather fat core, and thin up to big customer projects built upon it. Customer projects usually expand the core. Overwriting of key features does not happen. I might add that the core has some rather bad parts that are in urgent need of refactorings. There are specs, but mostly for the customer projects. The worst part of the core are untested (not as it should be...).

The developers are split into two teams, working with one or two PO for each sprint. Usually, one customer project is strictly associated with one of the teams and POs.

Now our problem: Rather frequently, we break each other's stuff. Someone from Team A expands or refactors the core feature Y, causing unexpected errors for one of Team B's customer projects. Mostly, the changes are not announced over the teams, so the bugs hit almost always unexpected. Team B, including the PO, thought about feature Y to be stable and did not test it before releasing, unaware of the changes.

How to get rid of those problems? What kind of 'announcement technique' can you recommend me?

Answer 1

I would recommend reading Working Effectively with Legacy Code by Michael C. Feathers. It explains that you really need automated tests, how you can easily add them, if you don't already have them, and what "code smells" to refactor in what way.

Besides that, another core problem in your situation seems a lack of communication between the two teams. How big are these teams? Are they working on different backlogs?

It's almost always bad practice to split up teams according to your architecture. E.g. a core team and a non-core team. Instead, I would create teams on functional domain, but cross-component.

Answer 2

We worst part of the core are untested (as it should be...).

This is the problem. Efficient refactoring depends heavily on suite of automated test. If you don't have those, the problems you are describing begin to appear. This is especially important if you use dynamic language like Ruby, where there is no compiler to catch basic errors related to passing parameters to methods.

Answer 3

The previous answers that point you to better unit tests are good, but I feel that there might be more fundamental issues to adress. You need clear interfaces for accessing core code from the code for the customer projects. This way if you refactor the core code without altering the behaviour as observed through the interfaces, the other team's code will not break. This will make it much easier to know what can be "safely" refactored, and what needs a, possibly interface breaking, redesign.

Answer 4

Other answers have highlighted important points (more unit tests, feature teams, clean interfaces to the core components), but there is one point I find missing, which is versioning.

If you freeze the behavior of your core by doing a release¹ and you put that release into a private artifact management system², then any customer project can declare its dependency on core version X, and it will not be broken by the next release X + 1.

The "announcement policy" then just reduces to having a CHANGES file along with each release, or having a team meeting to announce all features of each new core release.

Also, I think you need to better define what is "core", and what subset of that is "key". You seem to (correctly) avoid making many changes to "key components", but you allow frequent changes to "core". In order to rely on something, you have to keep it stable; if something is not stable, do not call it core. Maybe I could suggest calling it "helper" components?

EDIT: If you follow the conventions in the Semantic Versioning system, then any incompatible change in the core's API must be marked by a major version change. That is, when you change the previously existing core's behavior, or remove something, not just add something new. With that convention, developers know that updating from version '1.1' to '1.2' is safe, but going from '1.X' to '2.0' is risky and has to be carefully reviewed.

^{1: I think this is called a gem, in the world of Ruby
2: The equivalent to Nexus in Java or PyPI in Python}

Answer 5

Like other people said, a good suite of unit tests wont resolve your problem : you will have problem while merging changes, even if each team test suite passes.

Same for TDD. I dont see how it can solve this.

Your solution is non technical. You need to clearly define the "core" boundaries and assign a "watch dog" role to someone, be it the lead dev or architect. Any changes to the core must pass through this watchdog. He is responsible to make sure every output from all teams will merge without too much collateral damages.

Answer 6

As a more long term fix, you also need better and more timely communication between teams. Each of the teams that will ever utilize, for example, core feature Y, need to be involved in building the planned testcases for the feature. This planning, in and of itself, will highlight the different use cases inherent in feature Y between the two teams. Once how the feature should work is nailed down, and the testcases are implemented and agreed upon, there's an additional change in your implementation scheme that's required. The team releasing the feature is required to run the testcase, not the team that's about to use it. The task, if any, that should cause collisions, is the addition of a new testcase from either of the teams. When a team member thinks of a new aspect of the feature that is not tested, they should be free to add a testcase that they have verified passing in their own sandbox. In this way, the only collisions that will occur will be at the intent level, and should be nailed down before the refactored feature is released into the wild.

Answer 7

While every system needs effective test suites (which means, among other things, automation), and while these tests, if used effectively, will catch these conflicts sooner than they are now, this does not address the underlying problems.

The question reveals at least two underlying problems: the practice of modifying the 'core' in order to satisfy requirements for individual customers, and the failure of the teams to communicate and coordinate their intent to make changes. Neither of these are root causes, and you will need to understand why this is being done before you can fix it.

One of the first things to be determined is whether both the developers and the managers realize there is a problem here. If at least some do, then you need to find out why they either think they cannot do anything about it, or choose not to. For those who do not, you might try to increase their ability to anticipate how their current actions may create future problems, or replace them with people who can. Until you have a workforce that is aware of how things are going wrong, you are unlikely to be able to fix the problem (and perhaps not even then, at least in the short term.)

It may be difficult to analyze the problem in abstract terms, at least initially, so focus on a specific incident that resulted in a problem, and try to determine how it happened. As the people involved are likely to be defensive, you will need to be alert for self-serving and post-hoc justifications in order to find out what is really happening.

There is one possibility that I hesitate to mention because it is so unlikely: the customers' requirements are so disparate that there is insufficient commonality to justify shared core code. If this is so, then you actually have multiple separate products, and you should manage them as such, and not create an artificial coupling between them.

Answer 8

We all know that unit tests are the way to go. But we also know that realistically retro-fitting these to a core is difficult.

A specific technique that might be useful for you when extending the functionality is to try to temporarily and locally verify that existing functionality has not been changed. This can be done like this:

Original pseudo code:

def someFunction
   do original stuff
   return result
end

Temporary in-place test code:

def someFunctionNew
   new do stuff
   return result
end

def someFunctionOld
   do original stuff
   return result
end

def someFunction
   oldResult = someFunctionOld
   newResult = someFunctionNew
   check oldResult = newResult
   return newResult
end

Run this version through whatever system level tests that exist. If everything is fine, you know you have not broken things and can then proceed to remove the old code. Note that when you check the old and new results match, you might also add code to analyse differences to capture cases that you know should be different because of an intended change such as a bug fix.

Answer 9

"Mostly, the changes are not announced over the teams, so the bugs hit almost always unexpected"

Communications problem anyone? What about (in addition to what everyone else has already pointed out, that you should be rigorous testing) making sure that there is proper communication? That people are made aware that the interface they're writing to is going to change in the next release and what those changes will be?
And give them access to at least a dummy interace (with empty implementation) as soon as possible during development so they can start writing their own code.

Without all that, unit tests won't do much except point out during the final stages that there's something out of whack between parts of the system. You want to know that, but you want to know it early, very early, and having the teams talk to each other, coordinate efforts, and actually have frequent access to the work the other team is doing (so regular commits, not one massive commit after several weeks or months, 1-2 days before delivery).
Your bug is NOT in the code, certainly not in the code of the other team that didn't know you were messing around with the interface they're writing against. Your bug is in your development process, the lack of communication and collaboration between people. Just because you're sitting in different rooms doesn't mean you should isolate yourselves from the other guys.

Answer 10

Primarily, you have a communication problem (probably also linked with a team building problem), so I think a solution to your case should be focused on... well, communication, instead of development techniques.

I take for granted that it is not possible to freeze or fork the core module when starting a customer project (otherwise then you simply need to integrate in your company schedules some non-customer-related projects that aim at updating the core module).

So we're left with the issue of trying to improve the communication between the teams. This can be addressed in two ways:

• with human beings. This means that your company designates someone as the core module architect (or whatever lingo is good for the top management) who will be responsible for the quality and availability of the code. This person will incarnate the core. Thus, she will be shared by all the teams and ensure proper sync between them. Furthermore, she should also act as a reviewer of code committed to the core module to maintain its coherence;
• with tools and workflows. By imposing Continuous Integration on the core, you will make the core code itself the communication medium. This will require some effort first (by the addition of automated test suites on it) but then the nightly CI reports will be a gross status update of the core module.

You can find more about CI as a communication process here.

Finally, you stil have a problem with the lack of team work at the company level. I'm not a big fan of team building events, but this seems like a case where they would be useful. Do you have developer-wide meetings on a regular basis ? Can you invote people from other teams to your project retrospectives ? Or perhaps have some Friday evening beer sometimes ?