Overall speaking, code review is useful.

From time to time, my reviewers did catch some bugs that I overlooked, or point out options that fell into my blind spot. Sometimes I learn new things in code reviews. For example, last year a reviewer suggested me use ThreadLocal<T>, which indeed would simplify my code a lot. Some code reviews aren’t the code review per se, when it’s more about a FYI: “Hi, I have written this code and I like you to be aware of and get familiar with it.” That happens when you are the only person in the team truly understand that piece of legacy code. Although such FYI type of code review isn’t that much helpful to the author, it’s still good to the team.

But sometimes code review can become annoying, especially when people spend time on things that (in my opinion) don’t really matter. For examples:

• string.Join() or String.Join()?
• Put using directives inside or outside the namespace block?
• readonly static vs. const
• System.InvalidOperationException vs. just InvalidOperationException
• Optional parameter vs. overloading
• User var or explicit declaration?

I understand there are differences, often very subtle or trivial though, between string vs. String, readonly static vs. const, etc.. But those differences don’t do any real harm. Explicit declaration, such as Stopwatch stopwatch = Stopwatch.StartNew(), doesn’t make the code harder to read^[1] than var stopwatch = Stopwatch.StartNew(). String.Join doesn’t make the code slower than using string.Join. Putting using outside of the namespace block doesn’t make the code harder to work on. In addition, by default all versions of Visual Studio put using outside of namespace in the code it generates.

I really don’t like to spend time on those things in code reviews. I don’t think they matter to product quality and engineers’ productivity. There are so much more things that we wish we had more time to spend on to improve quality and productivity. Debating on those things are like debating whether to put one space after a period or two.

What people should do is to make sure that their team has collectively decided on what StyleCop^[2] rules to turn on in their code base and get included in the build. Once that’s decided and has taken effect, there will be no debate any more: if the rules are violated, it will be a build error and we don’t submit code review until it at least passes build. Simple and clear.

Huseyin Dursun, my previous manager, recently wrote a post “Automation eats everything …”, in which he pointed out that manual validation has been eliminated and technology companies are no longer hiring engineers exclusively for testing role. That’s exactly what happened last year in my group, Microsoft Azure. We eliminated test and redefined dev and now we only have software engineers, who write both product code and test code.

Now we have eliminated manual validation and all tests are automated. What’s next? My answer is: more automation. Here is a few areas that I see where we are/will be replacing other human work in the engineering activities with software programs.

1. Automation of writing test automation

Today, test automations are written by engineers. In the future, test automation will be written by software programs. In other words, engineers will write the code which writes test automation. One technique to consider is the model based testing. The idea of MBT has existed for nearly two decades and some companies (including teams in Microsoft, including my own teams) have tried and have got some successes. But by and large, it’s very under-used, mainly because other things aren’t there yet, like the scale, the demand, the maturity in other engineering activities^[1], the people, etc..

Another direction that people have been pursuing for at least a decade is the traffic bifurcation. The idea is to run the test instance as a shadow copy of the production instance, duplicate the production traffic to the shadow copy and see if it handles it in the same way as the production copy does. The bifurcation could be real time, or more in a record-and-replay fashion. Twitter’s Diffy is the latest work that I have seen in this direction. I guess there is a long way to go, especially when the SUT is very much stateful and its state has strong dependencies with the states in other downstream systems.

2. Behavioral contract enforcement

Using contracts to define system boundary and doing implementation against contracts is now very common. However, our contracts are mostly about the data schema: the API signature, the structure of the JSON object in the input parameters and response bodies, the RESTful API URL, the WSDL for XML Web Services, file format, response codes and error codes, … These contracts don’t carry much information about the behaviors: how will the entity transit through its state machine, whether an operation is going to be idempotent, whether I must call connection.Open() before doing anything else with it, etc.. In particular, the behaviors related to time. For example, this asynchronous operation is supposed to complete within N minutes; the system will perform this recurring operation every X days; …

Today the behavioral contracts are mostly written (if ever written) in our natural languages in design specifications. The enforcement of such behavioral contracts are done in automated test cases. But there could be some fatal gaps in today’s way. Our natural language is ambiguous. Test cases may not cover 100% what’s written in and implied by the design specification. A more fundamental challenge is that the intention of the automated test cases may drift away as time goes by, meaning: our test automation code use to be able to catch a code bug, but after test code changes and refactoring, one day it will no longer be able to catch the same bug. I don’t think we have a good way to detect and prevent such drift.

I believe the direction is to write the behavioral contract with some formal language, such as the TLA+ specification language created by Leslie Lamport. In a presentation last year, he explained how TLA+ works and how it’s used in some real work. It seems pretty intriguing.

3. Automation of the analysis

In my previous team, as we made the automated tests faster, we found that now the long pole became the time human spent to make sense of the test result. So we developed some algorithms and tools to help us: 1) differentiate whether a failure is a new regression, or just a flaky test, 2) which failed tests are likely to share the same root cause. That was very helpful. In addition, we plan was to totally get rid of signoffs and let the software programs to make the call most of the time.

4. Automation of the workflow

Ideally once my code has left my desktop, the entire desktop-to-production journey should be led by software programs with no human participation (except for intervention/override). Today some companies are closer to that dream (e.g. Netflix’s Spinnaker) and some other companies are farther away. Some smaller/simpler products may have already achieved it, but it remains a challenging thing for complex products. Today CI/CD is a lot more common in the software industry than ten years ago. But in my eyes today’s CI/CD tools and practices more like the DHTML and AJAX things circa early 2000’s. The jQuery/Bootstrap equivalent in CI/CD has yet to come.

5. Integration test in production

Besides replacing more human work with software programs, there is one more thing that we can do better in the test engineering: eliminate the test environment per se and perform all integration tests in production^[2]. Integration test is an inevitable^[3] phase between passing unit tests and getting exposed to real customers in production. Traditionally in integration tests, the SUT and most of its dependencies runs in the lab that are physically separated from the production instances. There are several big pain points in that approach: a) fidelity^[5], b) capacity, c) stability, d) support^[6]. Doing integration tests in production will make all these problems disappear. Needless to say, there are some challenges in this, mainly regarding product architect, security and compliance, isolation and protection, differentiation and equality, monitoring and alerting, etc.. I guess next time I will write a post about “The Design Pattern of Integration Testing in Production“.

[1] For example, a team should invest in other more fundamental things like CI/CD before investing in building the model and doing MBT.
[2] “Testing in production” is a highly overloaded term. Someone uses it to refer to A/B testing. Sometime it means a late stage quality gate where the new version is rolled out to a small % of production and/or exposed to a small % of customers. “Integration test in production” is different on two things: i) it’s for low quality code that is still under development, ii) it doesn’t get exposed to customer.
[3] There are some strong opinions against integration tests. The lines like “integration test is a scam” help highlight some valid points. But practically we shouldn’t throw the baby out with the bath water. I am strong believer of “pushing to the left” (meaning: put more tests in unit test and find issues earlier) but I too believe integration test has its place in the outer loop^[4]. Even though in the hindsight it might be very obvious that some bugs could have been caught by unit test, it would be a totally different thing when these bugs were unknown unknown.
[4] Outer Loop is defined as the stage between when an engineer has completed their check in and when it has rolled out to production. Depending on the product, this could mean App Store deployments (Mobile) or worldwide exposure (Services and modern Click to Run applications).
[5] Lab is different than production in many ways: configurations, security settings, networking, data pattern, etc. Those differences often hide bugs. Lab doesn’t have all the hardware SKUs that production has, which significantly limits how much we can do in the lab in hardware related testing (e.g. drivers, I/O performance, etc.).
[6] Let’s say the SUT depends on another service Foo. So traditionally in the integration test, we also have Foo instance(s) running in lab, too. When the lab instance(s) of Foo has any issue, the team of SUT will need the team of Foo to help check/fix. But that would be a lower priority for the team Foo, compared to the issues in the live site (production). Plus, the SLA (service level agreement) for lab instances is usually less than 24×7, but we want our integration tests to run all the time.

Last year in Windows Azure^[1], we merged dev and test^[2] and switched to the combined engineering model^[3].

Recently I have been asked quite a few times about my view of that change. My answer was: it solved a few chronic problems in the traditional dev+test model. It solved these problems fairly easily and naturally. If we didn’t do the combined engineering change, these problems would still be here today:

1. Quality is everyone’s responsibility

We always said: quality is owned by everybody, not just the test team. In the reality, there were always some gaps, more or less. Some developers still had the mentality of “the test team would/should find the bug for me”. Now there is no test team. Software engineers can count on nobody but themselves.

2. Improve testability

Although nobody disagreed with the importance of testability design, often times testability is treated as relatively lower priority by the developers in the traditional dev+test model. When they were under the time pressure, they naturally get the feature implemented first and it took long time for some testability requirements getting honored. The worse was that the developers didn’t have the sense of testability in their mind when they design and write code. Quite some testability issues were found in pretty late stage when it’s too costly/risky to change the design and code.

Now writing test code is a part of the software engineer’s job. They have much strong incentive to improve testability because it will make their own work easier. Plus, they truly learn the lessons of poor testability designs because it hurts themselves.

No more begging to the developers to add an API for my test automation to poll to replace a hard-coded Sleep(10000).

3. Push tests to the left

I had hard time to convince some developers to write more unit tests. This is a true story: a dev in my team wrote a custom lock. I found that there was little unit test of that lock. I asked the dev. He told me he think the scenario tests^[4] has already covered it pretty well. I didn’t know what to say. Yes we had code coverage data for unit test. But the hall of shame can only go this far.

Now developers (software engineers) own all the tests. Now they have all the incentives to push the tests to the left^[5]: put as much as tests in unit test, because it’s fast, easy to debug and nearly free of noises. The integration test is obviously a less favorable place to put the test: it’s slow, more hassle to debug and more noisy.

4. Hiring and retention

That was really, really, really a challenge all the time. Most college graduates prefer SDE than SDET^[6]. Partly because they had little exposure to what the SDET job is about. Partly because they are concerned with the “test” tag. Valid concern. Among the industry candidates, many of those who came from software testing background usually didn’t meet our requirement of coding and problem skills, because in many places outside Microsoft, test engineers were mainly doing what the STE^[7] used to do in Microsoft. We ended up having to put a lot of effort in convincing developers from other companies to join Microsoft as SDET, which wasn’t an easy sell.

Now, voila, problem solved. There is no more “test” tag. Everyone is “Software Engineer”. No more SDET wants to switch to SDE to get rid of the “test” tag, because there is no more SDET.

5. Planning and resourcing

We used to do our planning based on dev estimate only. It was understandable. It’s much messier to juggle if every work item has two prices (dev estimate and test estimate). In planning, we assume that for every work item, the test estimate is proportional to the dev estimate (e.g. 1:2, which came from our total test:dev ratio) and we believe the variances in each individual work item will average out. It worked OK most of the time. But there were several times where such model cause significantly under-funded test resources and caused crunch in late stage in the project.

Now when engineering managers and software engineers provide work estimate, the price tag has already included both dev estimate and test estimate. Nobody would underestimate the test cost because they would have to pay for it anyway.

To summarize, that’s the power of the roles and responsibility model. In the past, I was the cook at our home and my wife usually do the cleanup. She always complained that I made the stove and counter-top very messy. Later we made a change: I do both cooking and cleanup (and she took some other housework from me). Then all of sudden I paid a lot of attention to not make kitchen messy because otherwise it would be myself that spend time to clean it up.

p.s. Of course there is also the downside of this change. That would be another topic. But the net is a big plus.

[1] I know I should have called it “Microsoft Azure” rather “Windows Azure”. It’s just the old habit. For us who joined Azure in its early years, we still call it Windows Azure.
[2] Before the merge, we had dev team and test team. Take myself as an example. I was the test manager leading the test team, partnering with the dev manager who led the dev team. My test team was about half of the size of the dev team. In the shift to combined engineering model, we simply merged and became one engineering team of about 70+ people.
[3] Strictly speaking, our shift to the combined engineering did not only include merging the dev and test, but also redefined the role of PM, which now lean toward the market, customer and competition more than the internal engineering activities, and enlarged the role of the new “software engineer” role （which started from the sum of original dev+test) by adding more DevOps responsibilities.
[4] We didn’t differentiate these terms: scenario test, functional test, e2e test, integration test. Our dev did help write quite some functional/scenario tests when test team was running tight. But by and large, the test team owned everything after unit test.
[5] We usually draw a timeline on the whiteboard, from left to the right: the developer changes code in his local repo -> unit test -> other pre-checkin tests -> checkin -> integration tests -> start production rollout -> rollout completed. So “push tests to the left” means push them into the unit test.
[6] SDE = Software Development Engineer. SDET = Software Development Engineer in Test (aka “tester”).
[7] STE = Software Test Engineer. Microsoft had this job title until 2005/2006. STE’s main job responsibility was writing test spec, enumerating test cases, execute test cases (mainly manually), exploratory tests, etc.. Many STEs had very good analytical skills, knowledgeable of our product and good soft skills, but relatively weak in coding, debugging, design, etc..

One of my previous managers used to tell us “You either pay now or pay later. If you pay later, you pay more”. Years have passed and I have seen how true it is for an engineering team.

The dilemma is: the one who chooses not to pay now may not be the same one who pays later. Why would I pay now, so that someone else wouldn’t pay more later? It’s natural thing that we make selfish choices, unless there is something to counter balance it.

Here is an example, a real live site incident that happened recently. Our customer couldn’t start the virtual machine from the management portal. The cause was in the following code, it threw NullReferenceException because roleInstance.Current was null:

foreach (RoleInstance roleInstance in this.RoleInstances)
{
    int currentUpdateDomain = (int)roleInstance.Current
                                               .Container
                                               .ServiceUpdateDomain;
    //...
}

When the developer pressed “.” after roleInstance.Current, he probably didn’t pause and ask himself: would the Current always be not null? He probably didn’t spend time to read the related code a bit to find out and put extra code there for safety (e.g. “if(roleInstance.Current!=null)“). If he did all these (the pause, the code reading and the additional code), he would be slower. But that would have saved so much more associated with the live site incident: people’s time spent on investigate the incident, the time to rollout the hotfix, and the time to handle the (unhappy) customer. But those time is not the developer’s time. By cutting some corners, he probably got a few more work items done. Thus, he probably got a somewhat better performance review and promoted a bit sooner. Then he moved on and leave the team behind to “pay later but pay more”.

Our performance review model doesn’t help, either. In the annual review cycle, we barely can hold people accountable for something they did more than a year ago. Once bonus are paid and promotions are done, unless it’s something really bad (like causing the subprime crisis), we are not going to take the bonus back or revert the promotion.

Among the things that we can do, one thing I did was to keep my team members’ ownership unchanged^[1] for long time (e.g. two years, if not more) and told them so upfront. The benefits are:

• By fixing people on the same thing for longer time, the one who chooses not to pay now would be more likely the same person who will pay later (and pay more).
• By telling them so upfront, it does not only counter-balances the shortsighted cutting-corners, but also encourages the right behavior and investments in their areas that will lead to long-term successes. It’s like if I know I am going to live in this house for at least five years, I will spend the first year cleaning up the weeds and fixing the irrigation system in the backyard, then plant the plum trees in the second year and keep fertilizing and take good care of it in the third and fourth year, so that from the fifth year onward, I get to eat the sweat plums while enjoying the sun and breeze in my backyard.

That’s why re-org has a downside. In some companies where a re-org happens every 18-24 months, although the organizations get to more frequently optimize their structure and alignment, it also sets a norm that discourage long-term investments and successes: why bother planting the plum trees if I know I am going to move to another house every 18-24 months?

As Reid Hoffman said: “Good managers know that it’s difficult to achieve long-term success without obtaining long-term commitments from employees.”