×
Community Blog PouchContainer Engineering Quality Practice

PouchContainer Engineering Quality Practice

As PouchContainer develops, consistent code specification is needed to keep the code maintainable. This article shares PouchContainer practices in cod...

As PouchContainer keeps iterating and improving functions, the project scale grows larger, attracting many external developers for project participation. Because coding habits vary among contributors, code reviewers shall pay attention to the coding style in addition to logic correctness and performance, because consistent code specification is a premise for keeping project code maintainable. In addition to a consistent coding style, the coverage rate and stability of test cases are also the project focus. How can we ensure each code update has zero impact on existing functions in a project without regression test items?

This article shares PouchContainer practices in coding style specification and Golang unit test cases.

1. Consistent Coding Style Specification

PouchContainer is a project constructed using Golang. It uses shell scripts to complete automatic operations such as compiling and packaging. In addition to Golang and shell scripts, PouchContainer includes many Markdown documents to help users understand PouchContainer. The standard typography and correct spelling of the documents are the focus of projects. The following describes the tools and use cases of PouchContainer in terms of coding style specification.

1.1 Golinter – Keeps a Consistent Coding Format

Golang has simple syntax, and the complete CodeReview guide of the community from the start helps achieve a consistent coding style across many Golang projects and minimize disputes. Based on the conventions in the developer community, PouchContainer defines specific rules for developers to follow, so as to ensure code readability. For more information, read the code style rules.

However, it is difficult to keep a consistent coding style for projects solely based on written specification. Similar to other programming languages, Golang provides basic tool chains such as golint, gofmt, goimports, and go vet used to check and unify the coding style, making it possible to automate code review and subsequent processes. Currently, PouchContainer runs the preceding code check tools in CircleCI to check every pull request submitted by developers. If an error is returned, the code reviewer can reject review and code merge.

In addition to the tools provided by Golang, we can select third-party code check tools such as errcheck in open source communities to check whether developers have handled the errors returned by functions. However, these tools lack a consistent output format, making it difficult to normalize the outputs of different tools. Open source communities provide gometalinter to normalize various code check tools. The following combination is recommended:

  1. golint - Google's (mostly stylistic) linter.
  2. gofmt -s - Checks if the code is properly formatted and could not be further simplified.
  3. goimports - Checks missing or unreferenced package imports.
  4. go vet - Reports potential errors that otherwise compile.
  5. varcheck - Find unused global variables and constants.
  6. structcheck - Find unused struct fields
  7. errcheck - Check that error return values are used.
  8. misspell - Finds commonly misspelled English words.

A gometalinter package can be tailored to specific projects.

1.2 Shellcheck – Reduces the Potential Problems of Shell Scripts

Despite powerful functions, shell scripts require syntax check to avoid potential and unpredictable errors. For example, unused variables may be defined. Though such variables do not affect the use of scripts, they may be a burden on project maintainers.

#!/usr/bin/env bash

pouch_version=0.5.x

dosomething() {
  echo "do something"
}

dosomething

PouchContainer uses shellcheck to check the shell scripts of the current project. Take the preceding code as an example, shellcheck generates an alarm about unused variables. The shellcheck tool can identify the potential problems of shell scripts during code review to reduce the error probability during execution.

In test.sh line 3:
pouch_version=0.5.x
^-- SC2034: pouch_version appears unused. Verify it or export it.

The current continuous integration task of PouchContainer scans the .sh scripts of the project and uses shellcheck to check the scripts one by one. For more information, read the shellcheck documentation.

Note: When shellcheck is needlessly rigorous, you can add comments to the project to disable checking in expected places, or disable a check item. For specific check rules, check this wiki.

1.3 Markdownlint - Keeps Consistent typography

As an open source project, PouchContainer attaches equal importance to documents and code, because documents are the optimal way users can understand PouchContainer. Documents are prepared using Markdown, and their typography and spelling are the project focus.

Written specification is not enough to avoid false negatives in document checking, just like in the case of code checking. Therefore, PouchContainer uses markdownlint and misspell to check the typography and spelling of documents. Such checking is as important as golint and is performed on each pull request in CircleCI. If an error is returned, the code reviewer can reject review or code merge.

The current continuous integration task of PouchContainer checks the typography and spelling of the Markdown documents in a project. For configuration details, read here.

Note: When markdownlint is needlessly rigorous, you can disable check items in the project. For specific check items, read the markdownlint documentation.

2. How to Compile a Golang Unit Test

A unit test ensures the correctness of a single module. In a test pyramid, a unit test with wider coverage of more functions is more likely to reduce the debugging costs of integration testing and end-to-end testing. In a complex system, a longer link of task processing results in a higher cost of problem locating, especially problems caused by minor modules. The following lists the conclusions on how to compile Golang unit test cases in PouchContainer.

2.1 Table-Driven Test – DRY Principle

Simply put, a unit test is intended to determine whether the output of a function meets expectations based on a given function input. When a tested function has various input scenarios, we can organize test cases in Table-Driven mode. See the following code. Table-Driven uses arrays to organize test cases, and verify the correctness of functions by means of cyclic execution.

// from https://golang.org/doc/code.html#Testing
package stringutil

import "testing"

func TestReverse(t *testing.T) {
    cases := []struct {
        in, want string
    }{
        {"Hello, world", "dlrow ,olleH"},
        {"Hello, world", "dlorw ,olleH"},
        {"", ""},
    }
    for _, c := range cases {
        got := Reverse(c.in)
        if got != c.want {
            t.Errorf("Reverse(%q) == %q, want %q", c.in, got, c.want)
        }
    }
}

To debug and maintain test cases with ease, we can add auxiliary information to describe the current test. For example, when reference tests the input of punycode without adding punycode, the code reviewer or project maintainer may not know the differences between xn--bcher-kva.tld/redis:3 and docker.io/library/redis:3.

{
        name:  "Normal",
        input: "docker.io/library/nginx:alpine",
        expected: taggedReference{
            Named: namedReference{"docker.io/library/nginx"},
            tag:   "alpine",
        },
        err: nil,
}, {
        name:  "Punycode",
        input: "xn--bcher-kva.tld/redis:3",
        expected: taggedReference{
            Named: namedReference{"xn--bcher-kva.tld/redis"},
            tag:   "3",
        },
        err: nil,
}

For a function with complex behaviors, one input is not enough for executing a complete test case. In TestTeeReader, for example, data reading is complete after TeeReader reads hello, world from the buffer, and further reading is expected to encounter an "end-of-file" error. Such a test case must be executed independently rather than using Table-Driven.

Simply put, if you copy a large portion of code when testing a function, in principle, the expected test code can be fully extracted and used to organized test cases in Table-Driven mode. Be sure to follow the "Don't Repeat Yourself" rule.

Note: Table-Driven is recommended by the Golang community. For more information, this wiki.

2.2 Mock – Simulates External Dependencies

Dependencies are frequently encountered during testing. For example, a PouchContainer client requires an HTTP server. However, such dependencies exceed the processing capability of units and fall in the integration test scope. How can we complete these unit tests?

In Golang, interface implementation belongs to the Duck Type. An interface can be implemented in various modes, provided that the implementation complies with the interface definition. If external dependencies are subject to interface-based constraints, such dependency behaviors are simulated in unit testing. The following describes two common test scenarios.

2.2.1 RoundTripper

PouchContainer client testing is used as an example. The PouchContainer client uses http.Client. http.Client uses the RoundTripper interface to execute an HTTP request. With this interface, developers can customize the logic of sending HTTP requests, which is the major reason why Golang provides full support for HTTP 2 on the original basis.

http.Client -> http.RoundTripper [http.DefaultTransport]

For the PouchContainer client, the test mainly verifies the input destination address and the query, and determines whether results are returned normally. Therefore, before testing, developers must prepare corresponding RoundTripper, which only determines whether the input meets expectations, but does not implement the actual service logic.

In the following code, PouchContainer newMockClient can receive custom request processing logic. In a case which tests image deletion, the developer configures the custom logic to verify the destination address and determine whether the HTTP method is DELETE. In this way, the expected functional test can be completed without starting the HTTP server.

// https://github.com/alibaba/pouch/blob/master/client/client_mock_test.go#L12-L22
type transportFunc func(*http.Request) (*http.Response, error)

func (transFunc transportFunc) RoundTrip(req *http.Request) (*http.Response, error) {
        return transFunc(req)
}

func newMockClient(handler func(*http.Request) (*http.Response, error)) *http.Client {
        return &http.Client{
                Transport: transportFunc(handler),
        }
}

// https://github.com/alibaba/pouch/blob/master/client/image_remove_test.go
func TestImageRemove(t *testing.T) {
        expectedURL := "/images/image_id"

        httpClient := newMockClient(func(req *http.Request) (*http.Response, error) {
                if !strings.HasPrefix(req.URL.Path, expectedURL) {
                        return nil, fmt.Errorf("expected URL '%s', got '%s'", expectedURL, req.URL)
                }
                if req.Method != "DELETE" {
                        return nil, fmt.Errorf("expected DELETE method, got %s", req.Method)
                }

                return &http.Response{
                        StatusCode: http.StatusNoContent,
                        Body:       ioutil.NopCloser(bytes.NewReader([]byte(""))),
                }, nil
        })

        client := &APIClient{
                HTTPCli: httpClient,
        }

        err := client.ImageRemove(context.Background(), "image_id", false)
        if err != nil {
                t.Fatal(err)
        }
}

2.2.2 MockImageManager

For the dependency between internal packages, such as the dependency of PouchContainer Image API Bridge on PouchContainer Daemon ImageManager, the dependency behavior is subject to interface-based constraints. To test the logic of Image Bridge, similar to RoundTripper, we only need to execute corresponding Daemon ImageManager, without having to start containerd.

// https://github.com/alibaba/pouch/blob/master/apis/server/image_bridge_test.go
type mockImgePull struct {
        mgr.ImageMgr
        handler func(ctx context.Context, imageRef string, authConfig *types.AuthConfig, out io.Writer) error
}

func (m *mockImgePull) PullImage(ctx context.Context, imageRef string, authConfig *types.AuthConfig, out io.Writer) error {
        return m.handler(ctx, imageRef, authConfig, out)
}

func Test_pullImage_without_tag(t *testing.T) {
        var s Server

        s.ImageMgr = &mockImgePull{
                ImageMgr: &mgr.ImageManager{},
                handler: func(ctx context.Context, imageRef string, authConfig *types.AuthConfig, out io.Writer) error {
                        assert.Equal(t, "reg.abc.com/base/os:7.2", imageRef)
                        return nil
                },
        }
        req := &http.Request{
                Form:   map[string][]string{"fromImage": {"reg.abc.com/base/os:7.2"}},
                Header: map[string][]string{},
        }
        s.pullImage(context.Background(), nil, req)
}

2.2.3 Differences between ImageManager and RoundTripper

ImageManager and RoundTripper use the same simulation method, but the number of interface-defined functions varies between them. Normally, developers can manually define a structure that take methods as fields, as shown in the following code.

type Do interface {
    Add(x int, y int) int
    Sub(x int, y int) int
}

type mockDo struct {
    addFunc func(x int, y int) int
    subFunc func(x int, y int) int
}

// Add implements Do.Add function.
type (m *mockDo) Add(x int, y int) int {
    return m.addFunc(x, y)
}

// Sub implements Do.Sub function.
type (m *mockDo) Sub(x int, y int) int {
    return m.subFunc(x, y)
}

When interfaces are relatively large and complex, manual operation becomes a burden for developers during testing. To address this issue, the community provides automatic generation tools such as mockery to reduce the burden of developers.

2.3 Other tricks

In some cases, third-party services are the subject of dependency. For example, the PouchContainer client represents a typical case. Such testing can be completed using Duck Type. We can also register http.Handler and start mockHTTPServer to process requests. The preceding test method is cumbersome, and recommended for use only when testing cannot be completed by Duck Type, or it can be used in integration testing.

Note: In the Golang community, some attempts are made to complete monkeypatch by modifying binary code. We do not recommend using monkeypatch, and recommend that developers design and compile testable code.

Conclusion

Code style checking, unit testing, and integration testing must be performed by means of continuous integration during code review to help reviewers make accurate decisions. Currently, PouchContainer uses TravisCI or CircleCI and pouchrobot for code style checking and testing.

0 0 0
Share on

You may also like

Comments