From 17b305ce7f5b1b97022acf87671ed7eee549940d Mon Sep 17 00:00:00 2001 From: Kevin Diu Date: Wed, 13 Sep 2023 16:50:55 +0900 Subject: [PATCH] Update testing guideline for updated testing policy (#2131) * update testing guideline for updated testing policy Signed-off-by: kevindiu * style: Format code with gofumpt and prettier * fix grammar Signed-off-by: kevindiu * Update coding-style.md --------- Signed-off-by: kevindiu Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com> --- CONTRIBUTING.md | 22 +++++++-------- docs/contributing/coding-style.md | 45 +++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ce7c070e0f..91ea935bce 100755 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -120,7 +120,7 @@ To contribute unit test code, the steps are almost the same as [contribute sourc 1. Execute `make gotests/gen` command under Vald repository 1. Move the test function which you would like to implement above the placeholder `// NOT IMPLEMENTED BELOW` -1. Implement the test function +1. Uncomment and implement the test function Vald implmentes unit test code to ensure the quality of Vald. Each implementation file comes with its unit test file, with `*_test.go` postfix. @@ -132,10 +132,10 @@ func Test_implementedTest(t *testing.T) { } // NOT IMPLEMENTED BELOW - -func Test_notImplementedTest(t *testing.T) { - // this unit test function is not implemented yet -} +// +// func Test_notImplementedTest(t *testing.T) { +// // this unit test function is not implemented yet +// } ``` Vald defines a placeholder `// NOT IMPLEMENTED BELOW` to separate the implemented unit test from the unimplemented unit test. @@ -147,13 +147,13 @@ If no test functions are implemented in the test file, the placeholder will be p package test // NOT IMPLEMENTED BELOW -func Test_unimplementedTest(t *testing.T) { - // this unit test function is not implemented yet -} +// func Test_unimplementedTest(t *testing.T) { +// // this unit test function is not implemented yet +// } -func Test_unimplementedTest2(t *testing.T) { - // this unit test function is not implemented yet -} +// func Test_unimplementedTest2(t *testing.T) { +// // this unit test function is not implemented yet +// } ``` If all test functions are implemented, the placeholder will be placed on the bottom of the file. diff --git a/docs/contributing/coding-style.md b/docs/contributing/coding-style.md index 50a5ee693d..f8c0e0ab83 100644 --- a/docs/contributing/coding-style.md +++ b/docs/contributing/coding-style.md @@ -1363,3 +1363,48 @@ tests := []test { } } ``` + +## Testing policy + +In Vald, the implementation code is divided into different packages based on the context of the implementation. + +Based on the context of the package, we decided to apply different testing policies on each package to fit their needs. + +### Internal package + +Internal package `./internal` contains internal used library code. It is written to be reusable to solve common problems. + +It is very important to make sure that the commonly used library code works correctly, so we decided to apply C1 coverage (branch coverage) to the `./internal` package. + +C1 coverage means to cover the test on each condition and ensure each condition (true and false) is evaluated on the test code. + +### Pkg package + +Pkg package `./pkg` contains the business logic of the component in Vald. + +In the `./pkg` package, the business logic implementation is divided into each component, and then in each component package, the specific business logic is divided into different packages. + +Here is a common example of the package structure in `./pkg` package. + +- ./pkg/{component}/config +- ./pkg/{component}/handler +- ./pkg/{component}/router +- ./pkg/{component}/service +- ./pkg/{component}/usecase + +For example, the implementation of the use case layer of the Vald LB gateway will be `./pkg/gateway/lb/usecase`. + +Since each package has its purpose, we decided to apply different strategies to each package to fit its purpose. + +| Package | Testing strategy | +| ------------------------------ | -------------------------------------------------------------------------------------------------------------- | +| ./pkg/{component}/config | Only test if config file can be read and the corresponding value is set | +| ./pkg/{component}/handler/rest | No need to test | +| ./pkg/{component}/handler/grpc | Basically no test, other than testing bugfix (Detailed tests of business logic should be written in E2E tests) | +| ./pkg/{component}/router | No need to test | +| ./pkg/{component}/service | Test only interface functions | +| ./pkg/{component}/usecase | Test only New() function | + +For the rest of the `./pkg` packages, we decided to implement the unit test for the exported function only. + +Please follow the [unit test guideline](./unit-test-guideline.md) for more details on how to implement good unit test.