Update testing guideline for updated testing policy (#2131)

* update testing guideline for updated testing policy Signed-off-by: kevindiu <kevin_diu@yahoo.com.hk> * style: Format code with gofumpt and prettier * fix grammar Signed-off-by: kevindiu <kevin_diu@yahoo.com.hk> * Update coding-style.md --------- Signed-off-by: kevindiu <kevin_diu@yahoo.com.hk> Co-authored-by: deepsource-autofix[bot] <62050782+deepsource-autofix[bot]@users.noreply.github.com>
vdaas · Sep 13, 2023 · 17b305c · 17b305c
1 parent 631e1ff
commit 17b305c
Show file tree

Hide file tree

Showing 2 changed files with 56 additions and 11 deletions.
diff --git 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
@@ -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.