My coworker commits and pushes without testing

Question

When my coworker thinks that there is no need for a test on his PC, he makes changes, commits and then pushes. Then he tests on the production server and realizes that he made a mistake. It happens once in a week. Now I see that he made 3 commits and and pushes with deployment to the production server within 5 minutes.

I told him few times that this is not the way how good work is done. I don't want to be rude to him again and he is in the same status with me in the company and he has worked more than me here.

I want this behavior to be punished in some way or make it unpleasant as much as possible.

Before I started, the company was deploying using antique methods, such as FTP, and there was no version control.

I forced them/us to use Git, Bitbucket, Dploy.io, and HipChat. The deployment is not automatic, someone has to login to dply.io and press the deploy button.

Now, how can I force them to not test on the production server? Something like HipChat bot can sense that there is repeated edits on the same line and send a notice to the programmer.

Answer 1

You need a proper Quality Assurance (QA) process.

In a professional software development team, you don't push from development right to production. You have at least three separate environments: development, staging and production.

When you think that you got something working in your development environment, you push to staging first, where each commit is tested by the QA team, and only if that test is successful, it gets pushed to production. Ideally, development, testing and pushing to production is done by separate people. This can be ensured by configuring your build automation system in a way that developers can only deploy from development to staging and that the QA team can only deploy from staging to production.

When you can't persuade management to hire someone to do your QA, then maybe one of you can play that role for the other. I never worked with diploy.io, but some build automation systems can be configured in a way that a user can deploy both from development to staging and from staging to production, but not do both for the same build, so a second person is always required (but make sure you have some backup people for times when one of you is absent).

Another option is to have your support staff do the QA. This might seem like additional work for them, but it also makes sure that they are aware of any changes to the application which can safe them some work in the long run.

Answer 2

You will probably want to get a dev server, and preferably a staging environment too. Nobody should ever be pushing from local to production except for their own personal website. Your deploy process should only support dev->staging->prod. You probably want someone responsible for signing off new releases - depending on the organisation, this can be a project lead, a dedicated QA or a duty that rotates each week (with a tangible reminder e.g. only the person with the fluffy toy that week gets to push). However, discuss it with your team first to get buy-in (see below).

I want this behavior to be punished in some way or make it unpleasant as much as possible.

You could have your test suite (you've got one of those, right?) include a check that determines if you are on a production server and, if it does, sends everyone in the office an email saying Looks like $username is testing on prod, watch out. Perhaps publicly shaming your colleague would be unpleasant. Or your could create technical restrictions like IP-banning your team from looking at prod (which you can lift but you have to justify).

I don't recommend it, though, you would look like the problem, not the person who is testing on prod and you could make yourself very unpopular with the people in the team who don't care.

Surely what you really want is not for this behaviour to be punished but for it to stop?

I forced them/us to use [...]

It's great that you are advocating workflow improvements, but it sounds like either you don't think much of your colleagues and/or that you don't have their full support. This is likely to result in colleagues half-heatedly interacting with the workflow, doing the minimum needed to get code onto production and not really following the spirit of the workflow, which is going to mean more time spent clearing up. And when you're spending more and more time clearing up the results of inadequate interaction with workflow (because nobody else cares, right?) everyone else will be questioning the workflow itself.

So start with a conversation.

Find out why it's happening (is your colleague's machine not as good for testing? Is your colleague uncertain with feature branches or stuck in an svn mindset where commit and push are the same?), explain why it's a problem for you that untested code goes on dev/staging/prod, and see if you can do something to change why it happens (your colleague will more likely do what you want if you've just made it nicer to test locally than if you've just berated them).

If you can't resolve it and it genuinely comes down to a difference of opinion, schedule a teamwide discussion in your next retrospective meeting, see what your colleagues do and think. Make your case, but listen to the consensus. Maybe your team says it's ok not to test textual fixes locally, and you just have a rule that no big features go onto dev untested. Write down in the meeting and read out what you collectively decide about what is allowed on each of the environments. Set a date in a couple of months to review it, maybe at a retrospective.

Answer 3

At work, we avoid this by using Gerrit. Gerrit is a code review system that acts as a gate to your main/production Git branch which is conventionally called "master". You have code reviews, right? It sounds like you personally do them exceptionally. With Gerrit, you push to a sort of staging branch which, after you and your coworker have executed the code review process satisfactorily, Gerrit then merges to your master branch. You can even hook Gerrit to a CI server to execute unit tests before anyone gets an email to review a change. If you don't have a server you can set up a CI tool on, Codeship has a nice free plan to use as a proof of concept.

Last, of course, is to automate the production deployments only from sanctioned build products that have survived the code review and unit testing. There are some cool technologies coming up for this, which I'll not mention for fear it would be flame bait.

Go to your boss with a solution. This one applies even to you, and is a way to improve everyone's code quality, not just punish your coworker.

Answer 4

I see this as a largely human problem - the process is there and the tools are there, but the process is just not being followed.

I agree with what others have said here, about the possibility that it's quite possible the developer in question is just stuck in an SVN mindset, and may well think that he is following the process.

I find that the best way to address this kind of issue, especially where there may not be a clear superior, does not revolve around punishment or removal of access - this just leads to resentment and as it sounds like you are the loud proponent of following the process (there's always one, and I've been 'that guy' before), you're the one likely to cop the most heat. It revolves around bringing people to agreement on the process first.

This is where structured meetings, like "lessons learnt"-type meetings after a major incident in production, can be very useful. Try getting everyone to agree, including the developer in question, that code review, unit testing, comprehensive testing, etc. all need to take place, every time (and you can start to introduce the idea of a staging environment here too). It shouldn't be hard, and it's very sensible. Then ask the team to come up with some solid rules together, on how it should be done. The more the developer who is causing the problem contributes, the more they will feel like adhering to the rules, and start to see why their behavior has been a problem.

The final point, is never to fall into the "well, it's better than it used to be!" trap. There is always room for improvement. It is common for people in the IT industry, in my experience, to start settling for what they've got, after some improvements. The settling then continues, until you're suddenly at a crisis point again.

Answer 5

This is not uncommon, particularly in small teams. Don't make a big deal about it, but make an informal rule:

Break the build, bring in donuts.

Either 1) You'll get donuts twice a week or 2) they will adhere to the standard.

Bring it up in a meeting. Not accusingly, don't mention anyone by name, but something similar to, "Since we introduced version control and deployment standards, things have become much easier and the server is up more of the time than it used to be. This is great! However it's still tempting to take a shortcut and submit without proper testing. It's a gamble, though, and our server is on the line. I suggest that from now on if any of us submits code that breaks the server, the person responsible brings in donuts for the team the next day."

Substitute something else for donuts if desired, and don't make it expensive - lunch for the entire team would be too much, but a $5 box of donuts or fruit/veggie tray, or chips and dip, etc, etc would probably be annoying enough to make them actually weigh the risk against the convenience of skipping testing without making it a burden that would push them away from the team or company.

Answer 6

Now, how can I force them...

Instead of forcing your colleagues, try making them see things from your perspective. This will make things much easier for everyone. Which leads me into...

I want this behavior to be punished in some way or make it unpleasant as much as possible.

Why is it a pain for you with problems on the live server, but not for this guy? Do you know something that he doesn't? What can you do to make him see things the way you do?

If you succeed with this, you will bring out the best in him and he will find solutions to the problem you never thought of.

In general, try to work together with people to solve problems rather than forcing them into processes that don't understand.

Answer 7

What's the worst that could happen? Do you have backups that are good enough so that a bug modifying random records in your database can be fixed by restoring an old version?

Let's say you have a bug where you use a record id, and by mistake the amount of a bill in cents is stored in a variable used for the record id. So if I pay $12.34 then the record with id 1234 will be modified. Can you recover from that, if the software runs for a few hours until the bug is detected? (If both the correct record and record 1234 are updated, you would only detect this when record 1234 is used, so this could go undetected for quite a while).

Now ask your highly motivated developer what he thinks about this. Obviously he cannot claim that he never makes mistakes, because he has done so in the past.

Answer 8

You clearly understand various possible process and technical solutions. The issue is how to manage the coworker.

This is essentially a change management exercise.

Firstly, I'd have a quiet word with the founder to make sure he/she is on board with your point of view. If there is a production outage, I would expect that the founder would be highly concerned about that and desire change.

Secondly, you work in a small team and it's likely worth trying to get the whole team engaged in process improvement.

Setup regular (for example weekly) process retrospectives. Have the whole team:

• Identify problems
• Volunteer ideas for improving work practices
• Engage in implementing those practices

It should follow naturally that the whole team then also helps ensure compliance with the improved practices.

Answer 9

I think you've identified a couple of problems:

1. It sounds like any code that gets checked can be trivially pushed to production by anyone who has the rights to check in code.

Frankly I think that setup is just insane. At a minimum the people who can manually trigger a push to production should be restricted to the set of people who can be trusted completely to thoroughly review and test all outbound changes (regardless of who has authored the changes) before triggering the push. Giving anyone with permission to check in code the power to also arbitrarily trigger a push to production is just asking for trouble. Not just from careless and/or incompetent developers, but also from disgruntled ones or malicious third-parties who happen to compromise one of your accounts.

And if you're going to use a push-button process to deploy every change that gets checked in, then you need to have a comprehensive suite of automated tests in place, and pushing the button needs to run them (and abort the deployment if any tests fail!). Your process should not allow code that hasn't even been superficially tested to reach the point where it's being deployed to the production system in the first place.

Your coworker has made a big mistake in terms of checking in code without testing it first. Your organization has mad a much bigger mistake by adopting a deployment process that allows untested code to reach production under any circumstance.

So the first order of business is to fix the deployment process. Either limit who can trigger a push to production, or incorporate a reasonable amount of testing into your automated deployment process, or both.

2. It sounds like you might not have set any clearly-defined development standards/principles.

In particular, it sounds like you're missing a clear "definition of done", and/or using one that does not include "the code has been tested" as a gating factor on checking code in/deploying code to production. If you had this, instead of just pointing out that "good code isn't produced this way" you could say "this code isn't meeting the minimum standards that we've all agreed upon, and it needs to be better in the future".

You should try to establish a culture that clearly communicates what's expected of developers and the standards and level of quality they're meant to uphold. Setting up a definition of done that includes at least manual testing (or preferably, automated testcases that can be run as part of the build/deploy process) will help with that. As can having consequences for breaking the build. Or more severe consequences for breaking the production system. Note that those two things should really be independent, and it should be completely impossible to break both the build and the production system concurrently (because broken builds should not ever be deployable).

Answer 10

You should integrate a Continuous Integration + Peer Review process in the company. Bitbucket makes it easy.

And +1 to the dev server suggested by other users.

Don't be rude with him, it will only hurt your work relationship.