Blog Details

Code Reviews in Test Automation

In the previous post, we conceded that test automation is a software development effort. However, the test automation team comprises of individuals from a different background.  We have test automation engineers who were Test analyst with coding background from varsity. We have others who joined testing without any formal varsity qualification but had lots of passion for testing. We also have test automation engineers who were snatched from software development space. Codeless test automation teams have no notion of code review if they do it will just be a  review. Let us put the following topics under the spyglass:

1. What is code reviews?
2. Code review Guidelines and Coding Standards
3. Code Review Process
4. Process and Tools Integration for code review.

 

1. What is code reviews?

Code review is an opportunity to read the code from other members of your team with an intent to verify that agreed coding standards, best practice and naming conventions have been adhered to by the team. It’s also an opportunity to learn from your colleagues.  It’s an iterative process in which the team determines what is being done well and can be done better. Code review is a mechanism to quantify the quality of your code. The image below was borrowed from a Dutch professional freelance translator Thom Holwerda.

Thom’s depiction of code quality is one of the creative ways I have come across by far. Senior Managers will appreciate it if code reviews are tracked properly and measured to show value add to the company.  One of the avenues that can be employed to achieve this goal is the defect taxonomy register. From this register, a radar plot can be created to measure if our targets are being met. The graph shows the percentage of deviations fixed from the total percentage of found deviations, see sample graph below.

The targets show acceptable percentage as agreed by the team and management. These metrics show how much of the deviations have been correct from a certain test suite under review. Only 23% of wrong naming conventions have been fixed. The acceptable fix percentage is 80%  as such the signoff on the regression suite might be delayed until the fixes are applied.  The defect taxonomy analysis graph shows deviations found during the code review. see picture below.

This graph can be used by the Test Automation Engineer(TAE) to gauge which area of his bad habits she.he needs to focus on.The defect category shown here are not exhaustive and are specific to each team as a result, they will vary from company to company The legends in the graph are explained as follows :

• Reusability Defects: indicates the developer did not put much effort into the design of the code due to pressure or lack of skill.
• Maintainability Defects: indicates the developers are working under pressure to deliver as such they do quick fix which causes technical debt. Another aspect it indicates is that the TAE still need training or mentoring.
• Logic Defects: Implies the developer lacks fundaments knowledge of basic programming.
• Naming Conventions Defects: suggests the developer disregards the coding standards or the bad habit is not yet subdued.
• Correctness Defects:  Indicates the TAE did not understand the test case or the specification.
• Completeness Defects: indicates the TAE did not cover all the specification due to several reasons, lack of skill, Technology limitations or tool limitation.

During code review, we search for patterns and anti-patterns. It is recommended not to dwell on once of occurrence in the code but a widespread behavior.

1.1 Why do Code reviews?

Code reviews can be done for different reasons. The reasons do not have to be a long list as long as they resonate with the team. If the “why” of the review is not addressed the whole effort can go pear shape real quick.  here are few reasons why code review is beneficial to the team.

• Improve quality of the team
• Improve quality of communication between teams
• Improve the quality of the code.
• Curb the technical debt in the codebase.
• Reduce rework.
• Reduce maintenance costs
• it used to find coding problems

1.2 How to do code review?

It is critical to keep in mind that the aim is to look for repeated common problems, not just once-off issues. Whether a formal code review is done or an informal one, it is highly recommended to infuse the following practice in the code review.

• Use a Checklist:  the Checklist will depersonalize the reviews because we point out compliance problems that the whole team agreed to not violating when the team was creating the checklist. A sample checklist from the internet can be used but it must be updated regularly and it must resonate with the team.
• Review as partners: code review is not a student-teacher arrangement because it is done amongst peers hence it is at times called peer review. A level of respect to both reviewer and author need to be maintained. Noone is infallible, No matter how good you think you are, deviations from the standards are bound to happen.
• Review in chunks: review the test case or component as soon it is assigned to you. It is recommended to review 100 – 200 lines of code at a time. More than this quota you are subjected to missing some of the violations in the code. Don’t postpone components till the backlog has piled up, as it will also demoralize the author.
• Insist on Finished Not Perfect: No code is perfect, But all code must be finished.No empty else block or switch statement without the “break” statement. No return statement with a null value.

1.3 How not to do Code review? 

Code review environment can be a bit fragile if some of the pet peeves are not communicated in a diplomatic manner. Nothing makes a person uncomfortable than a response that seems to be blown out of proportion. Some of the pet peeves to weed out are as follows but not limited to :

• Don’t focus on blame – Pointing fingers has never yielded any solution
• Don’t get emotional – deviations are being exposed not your style of coding.
• Don’t redesign or fix the code – this is not the opportunity to redesign the code it is not your responsibility.
• Don’t judge – We know you could have coded it in a much better fancy way. The review is not based on how you would have coded it but the checklist and standards.
• Don’t be Scarstic  – get to the point in a professional manner, example “line 234, a method returns a null value please rectify instead of I see some of your methods are loafing around.”

2.Code Review Guidelines  and Coding Standards

This segment of the code review depends on the test automation tool being used, Subsequently the scripting language being used by the tool. In South Africa, the leading tool in test automation is Microfocus  UFT. Selenium is gaining traction because of dev-ops although lean-FT is not far behind in raining to the occasion.  I will demonstrate few standards that touch on VBScripting and C# programming language because UFT uses it for some of the API custom codes. The code review can be split into three levels if the team is not composed of resources on the same level of programming experience.

2.1 Code Review Level 1.

Level 1 delineates the most obvious and commonly occurring defect indicators that are universally acknowledged by software expert as bad practice. They are easy to identify and are easier to correct.VBScript is one of the languages that is so flexible to a level that you will want to puke.

VBScript Example 1: having a script with multiple waits that exhibit a pattern of three groups.So where the wait is 2, 5,10 will be replaced by constant values.

'script speed calibration variables
const SHORT_WAIT =2
const MEDIUM_WAIT =5
const LONG_WAIT = 10
Browser("Characteristic Value Search").Page("Control Central").Frame("SA_CHAR").Sync
Browser("Characteristic Value Search").Page("Control Central").Frame("SA_CHAR").WebEdit("SA_CHAR:"&j&"$EFFDT").Click
wait(SHORT_WAIT) 
Browser("Characteristic Value Search").Page("Control Central").Frame("SA_CHAR").WebEdit("SA_CHAR:"&j&"$EFFDT").Set strCurrentDate
Browser("Characteristic Value Search").Page("Control Central").Frame("SA_CHAR").WebList("SA_CHAR:"&j&"$CHAR_TYPE_CD").Click
wait(SHORT_WAIT)
Browser("Characteristic Value Search").Page("Control Central").Frame("SA_CHAR").WebList("SA_CHAR:"&j&"$CHAR_TYPE_CD").Select "Interest Suppression"
Browser("Characteristic Value Search").Page("Control Central").Frame("SA_CHAR").WebList("SA_CHAR:"&j&"$CHAR_TYPE_CD").Click 
wait(LONG_WAIT)

A browser sync method can be used instead of waits. Level 1 example for C# code will be how one can append a string to an original string to form a large string.

var reason = "Please update the status value";
reason += "before you can assign the task";
reason += "to a user with higher privilege";

// better way of doing it

var smartReason = @"This is a well structured reason.
that is no hassle to code.
don't you agree?";

For the code shown previously a string builder could also be an elegant way of achieving the same goal but that for level3 example.Another idea will be replacing the hardcoded value with parameters if those values will change in future of appears several times in the code.

2.2 Code Review Level 2

Apart from the additional defect indicators, there are two major disparities between Level-1 and Level-2. Firstly, the two standards differ in level of expertise required in order to identify defects. The litmus used to detect defects in Level-1 are unmistakably easy to identify that anyone, regardless of programming experience, is able to perform a Level-1 assessment

Level 2 standards enhance on level 1 by adding an additional set of defect indicators to produce more comprehensive measure/ controls for software quality.The Level-2 standard, on the other hand, demand a seasoned Test Automation Engineer (TAE).Secondly, while level-1 is basically objective in its review. In level 2 active discussions are promoted between the reviewer and the script developer.

using System;
using System;
using system.Collection.Generic;
using system.linq;
using system.text;
namespace LoopyTasks
{ 
   Class Program
   { 
     static void main(String[] args)
     { 
       // for loop that create memory leaks
       int months = 12;
      for (int i=0;i!= months;i+=5)
        {
          console.writeline($"Month of a counted:{i});
         }
      }
    }
}

The code was intended to print the last month of the Quater because the increment will skip 12 the condition will never be met. Level 2 code review might require the code to be executed if reading the code won’t be sufficient.

2.3 Code Review Level 3

The level 3 guidelines will collate an extensive and up-to-date set of defect indicators, including those from level-1 and level -2. These indicators will be encapsulated in a comprehensive checklist for code walkthrough, used to perform an in-depth analysis of program source code.

Because level 3 standard will represent current industry best practice, it will be updated regularly in order to reflect any new research. Monthly review of the checklist is required.

In comparison with the other first two, Level 3 will be considerably more expensive and time-consuming to implement, since it requires a substantially a higher degree of programming expertise, and also because it requires two people to perform the assessment.

Level 3 will attempt to rectify the following defects in the source code :

• File Not Checked: external file being accessed without checking whether if the file is available or not first.
• External resources Not released: Ending database or external data source access without releasing or closing them properly.
• No Encapsulation:   Information only relevant to the internal computations of a module is available to other modules.
• Module Not Cohesive: Functions which are cohesive typically easier to modify, easier to fix and contains fewer errors compared to functions with varied tasks.
•  Nesting Too Deep: Robert C. Martin the author of “Clean Code” supports the notion that the minority people can comprehend nesting of conditional statements to more than 3 levels.
• Module Highly Coupled: Functions with low coupling are easier to modify without causing a huge impact on other modules and changes don’t introduce new bugs

OOP concept only applies to CSharp only but not to VBScript if the test automation team only ventures within the VBScript realm only they need not worry.

3. Code review Process

Code review process shows the status the component or test automation script might be in depending on the outcome of the review. The process below was designed with Microfocus ALM test management tool, but it can still apply to other test management tools as well.

The process starts by establishing if it is possible to automate a set test case or not. when it is possible the test case is scripted then the peer review status is set to not reviewed by default

4.Process and Tools Integration for code review.

Microfocus test automation IDE is not the best in the market, compared to most IDE that host selenium framework. They come with or have the capability to integrate with plugins design specifically for code reviews like static code analyzer. This plugin goes a long way in detecting code issues that will have to be detected manually using UFT.

Code review can still be performed seamlessly using Microfocus ALM together with UFT. Microfocus ALM is used to change the status of the component being scripted as well as assigning it to the relevant reviewer on the team see screenshot below.

The prior mentioned review status will be changed according to what was the outcome of the code review in UFT. In UFT GUI got to View>>Task. The task pane will appear at the bottom of the IDE. The task description and line number will not show any value until the prefix to do is added in the code followed by what needs to be done. see the snapshot below.

Line 54 tells you what needs to be done and it is shown in the task pane. If to do prefix is removed UFT will treat the comment as a comment that was meant for the code but not the author. This process is not as ideal as using dedicated collaboration tool for code review but it does work and is better than not having a workaround at all.