Anyone working as a software engineer has experienced diving into existing code written by another person or team. Despite many articles, courses, and blog posts that emphasize the need to write clean and tested code, the reality never really lives up to the idealistic picture presented to us. Inexperience and deadlines, among many other reasons, can leave code repositories with suboptimal documentation, resulting in a constant need for refactoring and a growing to-do list.
In this blog post, I would like to invite you to take a peek at the ongoing development of one such repository my team has inherited. I will describe the rules for code refactoring we established early on and the conclusions drawn later in development.
Ginkgo
Installation
Firstly, a bit about the code. The repository in question is a test suite written in Golang. It utilizes a testing framework called Ginkgo designed to help Go developers write expressive test cases. This pattern is often referred to as “behavior-driven development.” It builds upon the concept of test-driven development and encourages developers to discuss the behavior of applications and, via elaborate examples, implement test cases. Lastly, the actual logic is implemented in the backend, and the test cases serve the purpose of acceptance criteria.
Ginkgo builds on top of Golang’s existing test infrastructure, meaning that everything with regard to project structure and whether the test suite lives in the same package as the code itself or exists separately is absolutely fine and only depends on the suite's use case. Installation of Ginkgo, as in most cases for Golang, is simply done by invoking the go get command, like so:
go get github.com/onsi/ginkgo/v2
go install github.com/onsi/ginkgo/v2/ginkgo
After that, run “ginkgo bootstrap” in the package directory. This will create a new file called “<<package_name>>_test_suite.go” with bootstrap code that links the Gomega library’s assertions and Ginkgo’s fail handler.
Here, you can read more about our Golang development services, or if you want to learn about handling Golang errors, check out our previous article.
Writing test cases
Let’s use an example from Ginkgo’s documentation to go through what Ginkgo provides and how it works.
var _ = Describe("Books", func() {
var foxInSocks, lesMis *books.Book
BeforeEach(func() {
lesMis = &books.Book{
Title: "Les Miserables",
Author: "Victor Hugo",
Pages: 2783,
}
foxInSocks = &books.Book{
Title: "Fox In Socks",
Author: "Dr. Seuss",
Pages: 24,
}
})
Describe("Categorizing books", func() {
Context("with more than 300 pages", func() {
It("should be a novel", func() {
Expect(lesMis.Category()).To(Equal(books.CategoryNovel))
})
})
Context("with fewer than 300 pages", func() {
It("should be a short story", func() {
Expect(foxInSocks.Category()).To(Equal(books.CategoryShortStory))
})
})
})
})
Ginkgo uses several nodes to structure test cases: setup nodes, container nodes, and subject nodes. Container nodes in the above code would be Describe() and Context(). They allow developers to organize blocks of code hierarchically. We use setup nodes like BeforeEach() to prepare the state before each test case. Lastly, we can specify subject nodes with the It() block in which proper assertion can be made.
In this case, we are “describing” book categories in two separate “contexts” - for long and short stories. In BeforeEach(), we instantiated book variables and later compared the output of the book’s Category() method against correct enum values.
After running the test suite, Ginkgo will recognize the number of subject nodes and prepare a spec tree. Each spec can have any number of setup nodes but only one subject node. This means that having two subject nodes, as in the example above, will result in two specs being run. However, there is nothing stopping us from adding another setup node, like BeforeEach(), inside one of the Context() blocks.
Ginkgo comes with several other features and nodes that help write expressive test cases. In addition to Describe() and Context(), one can use When() if it better suits language semantics. To clean up after the suite has concluded, you can use DeferCleanup(), and you can pinpoint smaller steps in the It() block by adding the By() functions inside it.
Refactoring techniques
Refactoring is a process as old as programming itself. Because of this, an endless amount of articles and books have been written about techniques that can greatly help achieve so-called clean code.
Red-green refactor
The first technique refers to a three-step plan. The colors are symbolic of passing tests (green) and failing tests (red). The first step is the red phase: write failing tests that will serve as acceptance criteria later and look at what needs to be developed. Next, in the green phase, make the changes necessary to make the tests run successfully. After that, perform code refactoring. Hopefully, this won't change the outcome of the tests, but this is actually where the cycle repeats itself. New tests can be added, and the older ones make sure that nothing was broken or lost along the way.
Preparatory refactoring
Preparatory refactoring describes a refactoring process that takes place during the implementation of a new feature. That new feature could use some of the previously written functionalities or have them improved. In that case, we can prepare partially new implementation details for the new feature and gradually introduce them in the older code.
Refactoring by abstraction
In the case of large code refactoring, it may be good to take a step back and think about the architecture and structure of classes and methods and look for places that can reduce redundancy and duplicated code. The most known example of this technique is the Pull-Up and Push-Down method in which the developer should actively think at which level various methods and fields should be - superclass, subclass, separate function - and move them (pull down or up) accordingly.
Inheriting existing code
Let’s move to the main point of the article, which is working on inherited code. Our team was tasked with creating an end-to-end integration test framework for a service mesh tool. Some tests had already been written, and helper functions had been added. A few problems struck us immediately after looking at the existing code.
First of all, all test cases failed when run. There was no documentation added nor any comments regarding the purpose of objects and functions, and the test cases themselves didn't match the acceptance criteria provided by the client. It was obvious from the get-go that major code refactoring would be needed before any new test cases could be implemented.
General tips for Go refactoring
Make it run
The first steps are universal for any code repository. In the beginning, just make it run. Usually, it doesn't take a lot of effort, and with that, you can see what it does, read the logs the program produces, and get a general feeling of the tool at hand. Some quick bug fixes here and there, and it should feel much better knowing you got it up and running. In this case, we used this step to learn about the various features we were supposed to implement tests for and get in-depth answers from the QA and testing team overseeing this tool.
Once the code is functioning correctly, try not to make big changes during the refactoring process. By doing so, you run the risk of continuously fixing other parts of the code and making changes that are hard to review in one go. Making continuous but small changes will still yield the same results in time and it will be easier to review and track those changes. You can find some tips on generics in Go in our previous article.
Get to know the software and software development process
We also received a set of instructions: one for acceptance criteria for the test suites and another one for common scenarios in which a user might use the software. Getting to know the tool and taking time to learn both front and backend is another one of those generic pieces of advice, but it's something we can't forget.
Comments
Everything is clear when you're writing code. Every line has a specific thought behind it, and it's often not until a day or two later that you sit back down and think to yourself, "What did this line do?" Despite various sources advising on ambiguous code blocks, there are still places where the true intent is only known to the original writer.
Going step by step through the original code, we added comments (if needed) and a list of TODOs that jumped to mind. Seeing duplicated code was typically addressed immediately by moving functionality to separate code blocks.
Unit tests
This part is often very hard with corporate deadlines hanging over your head, but ultimately some downtime arises. By that time, you should recognize which parts need testing the most and there is no greater way of limiting technical debt than conducting some straightforward unit tests.
Golang-specific tips
Linters
Use linters. One of the first MRs was to add static checks and that alone increased readability a lot.
Don't reinvent the wheel
Have you ever read code and thought, “There’s probably a package for that”? Then there’s probably a package for that. While looking into possible imports is important, remember not to add new packages just for a small functionality that could be easily implemented directly in your repository.
Get to know the packages used in the project
Before my arrival on the project, I only knew the product's name and that we would be using the Ginkgo testing framework. Naturally, I jumped into the documentation and by the time I first laid my eyes on the existing code, I already had ideas on what to do. With that knowledge, my team and I noticed many areas in need of improvement and refactoring.
Actively think of project structure
Golang doesn't have an official standard project structure but some common project pattern layouts have emerged over time with many examples and ideas. Use them to your advantage and decide what best suits your project.
Code examples
Nothing better than actually seeing the fruits of your hard work, so I would like to show a few "before and after" code snippets.
Project structure
Keep in mind which functionalities and methods you would like to be exported and which ones are for internal use only. As I have already mentioned before, we had to write a test suite for the client's software. To do this, we had a cluster deployed with our own instance of the main application and on it, we could run a simple test service and check if the main application was properly interacting with the service (test app).
.
├── docker-compose.yaml
├── Dockerfile.qa
├── go.mod
├── go.sum
├── Makefile
├── pkg
│ ├── client
│ │ └── client.go
│ └── config
│ └── config.go
├── README.md
├── scripts
│ ├── install_ginkgo.sh
│ └── local_run.sh
└── mainapplicationverification
├── configtemplates //json directory
├── suites
│ └── feature1
│ ├── feature1_suite_test.go
│ └── feature1_test.go
└── testapps
├── testappname
│ ├── testappname.go
│ └── manifests
│ └── k8s_files.yaml
└── testapp.go
This setup in itself is fine. We have a pkg folder with general functionalities, scripts for developers to run to ease setting up prerequisites locally, and finally a directory for the main application that has space for test cases for its specific features and methods for running microservices in tests in the test apps directory.
The problem is that all setup with the main application was made in the pkg directory. Due to this, the previous developers made assertions using Gomega in these packages and that in turn is generally discouraged because the pkg directory is traditionally reserved for reusable packages that can be imported and used by other projects. Due to this, we restructured it a bit by adding several folders under the main application verification folder to make sure the assertion can reach as far in the stack trace as we want.
In the config templates, the directory was a set of JSON payloads for API requests in test cases, which was a lazy way of copying what the browser's developer tool provided instead of using Golang's encoding/JSON package that takes advantage of structs and tags. A quick search in the client's repository led us to specific structures generated from the tool's API for Golang. Replacing this directory with those structs allowed us to easily configure API requests and delete around 50 files that would otherwise have just kept stacking up once more suites were added.
Soon after receiving the repository it was obvious that one test app wouldn't be enough, but adding new ones would duplicate a lot of code as testapp.go was only suited for the one that was previously implemented. Thanks to preparatory refactoring and refactoring by abstraction we could move the test app's specific code to a lower level and make testapp.go an interface for all other test applications, and use proper getters to access fields with shared names among apps in test cases.
If your issues are private repositories, Go modules, Docker, and CircleCI, I recommend you read our previous article.
Ginkgo test code
The test cases themselves seemed to be a copy-pasted first example in the Ginkgo documentation rewritten to suit what the software’s developers needed at the time of writing the code.
var _ = FDescribe("Test cases", func() {
BeforeEach(func() {
GetApiToken()
CreatePrerequisiteResource()
time.Sleep(timeDuration)
})
AfterEach(func() {
DeleteResource()
})
When("Test case 1", func() {
SetupStep1()
SetupStep2()
It("Test case 1", func() {
err := TestAppWorksAsIntended()
Expect(err).NotTo(HaveOccurred())
})
})
When("Test case 2", func() {
SetupStep1()
SetupStep2()
SetupStep3()
It("Test case 2", func() {
err := TestAppWorksAsIntended()
Expect(err).NotTo(HaveOccurred())
})
})
When("Test case 3", func() {
It("Test case 3", func() {
err := TestAppWorksAsIntended()
Expect(err).NotTo(HaveOccurred())
})
})
})
After:
var _ = Describe("Test cases", Label("Test cases"), func() {
BeforeEach(func() {
GetApiToken()
CreatePrerequisiteResource()
VerifyResourceHasBeenCreated()
})
AfterEach(func() {
DeleteResource()
})
When("Test cases 1 & 2", func() {
BeforeEach(func() {
By("Setup 1", func() {
SetupStep1()
})
By("Setup 2", func() {
SetupStep2()
})
})
It("Test case 1", Label("Test case 1"), func() {
ExpectTestAppWorksAsIntended()
})
It("Test case 2", Label("Test case 2"), func() {
By("Setup 3", func() {
SetupStep3()
})
By("Expect that test app works as intended", func() {
ExpectTestAppWorksAsIntended()
})
})
})
When("Test case 3", Label("Test case 3"), func() {
By("Expect that test app works as intended", func() {
ExpectTestAppWorksAsIntended()
})
})
})
new file: assertions/test_app.go:
func ExpectTestAppWorksAsIntended() {
err := TestAppWorksAsIntended()
Expect(err).NotTo(HaveOccurred())
}
Firstly we changed FDescribe() to Describe(), as FDescribe is only used when you want to quickly run only this node, and delegated this functionality to labels which made it easier to single out the test case with a script. We also took advantage of the possibility of using many setup nodes to get rid of duplicate code in test cases 1 and 2. In the outermost setup node, we got rid of the ambiguous time.Sleep() line and extracted it to a setup function which allowed us to verify if resources were created. This function uses the built-in gomega.Eventually() function which enables making assertions on asynchronous behavior. We also prepared a separate file to make assertions in a suite to improve the expressiveness of the code in the test case file and lastly introduced By() clauses for better logging.
Summary
In conclusion, the world of refactoring is not a process that's easily described by a set of instructions that apply to any given problem. It requires preparation and discussion, often on a very abstract level, and constant awareness of the underlying context. Keep in mind there's always something that can be done better and it should be your judgment that constantly weighs code refactoring against implementation of new features. Don't fall into the trap of endlessly pursuing only one of these options.
