When developing an application in a team not everyone on this team will be equally good developers. Some will be skilled in some things and some in others and some not at all.
As a team working in an agile process you usually have a definition of ‘done’ to follow. In our DoD it says never to merge code that is not documented and have proper tests. Sometimes though, a feature is so critical to some team member so the code has to be merged without tests. Sometimes a lazy developer might merge and call it a weekend.
A merge like this means that when other team members pull the code, they will receive code that may or may not function properly and since there are no tests it is hard to verify. It can be very cumbersome to walk through that code to make sure it works before the team member can work on their task.
It has happened on several occasions that the code that was committed without tests has been broken by other team members without them noticing.
So my question becomes:
- How would you handle issues like this?
- Does it point to bad planning rather then bad behaviour?
- Who is to “blame” if code that lacks tests is broken?
- How would you handle issues like this?
Take a look at continuous integration. When people commit, there should be an automatic build that runs the unit tests. This will catch regressions introduced in the new commit. It won’t catch errors that have no tests – that will require changing behavior. Consider adding unit tests to your code review – if they don’t exist, they don’t commit.
For a specific case, I’d probably revert the merge (if possible) and let them do it again once the issue is resolved. If the revert isn’t possible, I’d have whoever pushed the commit drop everything until they fix what they committed. Remember, as long as the build is broken everyone is blocked.
- Does it point to bad planning rather then bad behaviour?
Not exactly. It might be bad planning in that there’s a rush to commit before something is done (it sounds like you define done as tested), but it’s more likely having a definition of done that doesn’t match your DoD (which is behavior).
- Who is to “blame” if code that lacks tests is broken?
The person who committed the coded and whoever approved the untested commit (reviewers, technical leadership, etc).
- How would you handle issues like this?
This Depends in what position you are. If you’re their boss – take actions to discipline then. If not, try to explain them, that working code ist the value of the company you’re in. Everybody must be aware of this fact. I’m quite sure they will recognize what’s wrong.
On the other hand, you should add the tests by yourself. If you don’t make sure it will be tested now it might never get tested. Because, according to LeBlanc, later equals never.
- Does it point to bad planning rather then bad behaviour?
Not if Tests has been included in the planning.
- Who is to “blame” if code that lacks tests is broken?
As stated by thegrinner – the person who commited the code.
I think I may have inadvertantly answered your question in another question. To summarize what I wrote there, it is always bad practice to commit broken code (or code which breaks tests) if other programmers often pull from that code repository. I think the proper approach is simply to expect that such things will happen, and set up the code repository in such a way that it won’t affect others regardless.
In other words, general practice should be to branch upon doing significant (or even insignificant in the interests of being thorough) changes and committing to the main trunk only when you’ve established that it won’t cause problems.
I would argue that, yes, this is a sign of bad planning rather than bad behavior. The only people to “blame” if code that lacks tests is broken is those who set up the code repository and/or set up the system for code submission. It is perfectly normal if programmers make mistakes. However, it is not acceptable to build a system that cracks when such mistakes are made.
How would you handle issues like this?
- Work on training the less good developers to be better.
- Work on educating management about the wasted time/money caused by bad commits and poor quality.
- Work on pushing technical solutions like gated check-ins or “only team leads can commit”
Does it point to bad planning rather then bad behaviour?
Sometimes. If a feature is so important due to time pressure or dependencies, that’s a planning issue. If the developer is lazy, or estimated it to take a week (with tests) and then cut the tests to meet the goal that’s not a planning (PM) issue really.
Who is to “blame” if code that lacks tests is broken?
Everyone. Even if one person pushed you too hard, you could still take the time to write the tests or push back on the schedule. Even if one person just was lazy, the culture/boss/peers let that happen and/or didn’t mitigate against natural human laziness.
Mistakes will be made, but complacency is what will really destroy your department. Acceptance of this issue and pushing the blame to others will not improve it.