Merge branch 'development' into eclesio/substrate-docker-image

.github/CODE_STYLE.md

@@ -6,3 +6,125 @@
 
 Add `String() string` methods to new types, so they can easily be logged.
 💁 You should try de-referencing pointer fields in your method, to avoid logging pointer addresses.
+
+## Subtests with mocks
+
+We use `gomock` to use `mockgen`-generated mocks.
+
+This is trivial to use with single test, but it can get tedious to use with subtests.
+
+In the following we use this example production code:
+
+```go
+//go:generate mockgen -destination=mock_multiplier_test.go -package $GOPACKAGE . Multiplier
+
+type Multiplier interface {
+  Multiply(n int, by int) (result int)
+}
+
+// Function we want to test
+func multiplyByTwo(n int, multiplier Multiplier) (result int) {
+  return multiplier.Multiply(n, 2)
+}
+```
+
+In your tests, since you need to define a controller
+
+```go
+ctrl := gomock.NewController(t)
+```
+
+before configuring your mocks, it means you must **create the controller and configure your mocks in your subtest** and not in the parent test. Otherwise a subtest could crash the parent test and failure logs will look strange.
+
+⛔ this is **bad**:
+
+```go
+func Test_multiplyByTwo(t *testing.T) {
+    ctrl := gomock.NewController(t)
+
+    multiplier3 := NewMockMultiplier(ctrl)
+    multiplier3.EXPECT().
+        Multiply(3, 2).Return(6)
+
+    testCases := map[string]struct {
+        n          int
+        multiplier Multiplier
+        result     int
+    }{
+        "3 by 2": {
+            n:          3,
+            multiplier: multiplier3,
+            result:     6,
+        },
+    }
+
+    for name, testCase := range testCases {
+        t.Run(name, func(t *testing.T) {
+            result := multiplyByTwo(testCase.n, testCase.multiplier)
+
+            assert.Equal(t, testCase.result, result)
+        })
+    }
+}
+```
+
+By default, you should aim to:
+
+1. Specify the mock(s) expected arguments and returns in your test cases slice/map
+1. Configure the mock(s) in your subtest
+
+Corresponding example test:
+
+```go
+func Test_multiplyByTwo(t *testing.T) {
+    testCases := map[string]struct {
+        n               int
+        multiplierBy    int
+        multiplerResult int
+        result          int
+    }{
+        "3 by 2": {
+            n:               3,
+            multiplierBy:    2,
+            multiplerResult: 6,
+            result:          6,
+        },
+    }
+
+    for name, testCase := range testCases {
+        t.Run(name, func(t *testing.T) {
+            ctrl := gomock.NewController(t)
+
+            multiplier := NewMockMultiplier(ctrl)
+            multiplier.EXPECT().
+                Multiply(testCase.n, testCase.multiplierBy).
+                Return(testCase.multiplerResult)
+
+            result := multiplyByTwo(testCase.n, multiplier)
+
+            assert.Equal(t, testCase.result, result)
+        })
+    }
+}
+```
+
+Now there is an exception where your mocks configuration change a lot from a test case to another. This is seen with **at least two levels** of `if` conditions nesting to configure your mocks. In this case, you shall avoid having a test cases structure (slice/map) and run each subtest independently. For example:
+
+```go
+func Test_(t *testing.T) {
+    t.Run("case 1", func(t *testing.T) {
+        ctrl := gomock.NewController(t)
+        // ...
+    })
+
+    // ...
+
+    t.Run("case n", func(t *testing.T) {
+        ctrl := gomock.NewController(t)
+        // ...
+    })
+}
+```
+
+💡 this is usually a code smell where the production function being tested is too long/complex.
+So ideally try to refactor the production code first if you can.

.github/workflows/build.yml

@@ -3,22 +3,24 @@ on:
   push:
     branches:
       - development
-    paths:
-      - .github/workflows/build.yml
-      - "**/*.go"
-      - "chain/**"
-      - "cmd/**"
-      - "dot/**"
-      - "internal/**"
-      - "lib/**"
-      - "pkg/**"
-      - "tests/**"
-      - .dockerignore
-      - .codecov.yml
-      - Dockerfile
-      - go.mod
-      - go.sum
-      - Makefile
+    # Commented paths to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # paths:
+    #   - .github/workflows/build.yml
+    #   - "**/*.go"
+    #   - "chain/**"
+    #   - "cmd/**"
+    #   - "dot/**"
+    #   - "internal/**"
+    #   - "lib/**"
+    #   - "pkg/**"
+    #   - "tests/**"
+    #   - .dockerignore
+    #   - .codecov.yml
+    #   - Dockerfile
+    #   - go.mod
+    #   - go.sum
+    #   - Makefile
 
 jobs:
   builds:

.github/workflows/checks.yml

@@ -1,16 +1,14 @@
 on:
   pull_request:
-    types:
-      - opened
-      - edited
-      - reopened
-    paths:
-      - .github/workflows/checks.yml
-      - .github/PULL_REQUEST/pull_request.go
-      - .golangci.yml
-      - "**/*.go"
-      - go.mod
-      - go.sum
+    # Commented paths to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # paths:
+    #   - .github/workflows/checks.yml
+    #   - .github/PULL_REQUEST/pull_request.go
+    #   - .golangci.yml
+    #   - "**/*.go"
+    #   - go.mod
+    #   - go.sum
 
 name: checks
 env:
@@ -31,7 +29,9 @@ jobs:
 
   check-description:
     name: Checks PR has title and body description
-    if: ${{ github.actor != 'dependabot[bot]' }}
+    # Commented to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # if: ${{ github.actor != 'dependabot[bot]' }}
     runs-on: ubuntu-latest
     steps:
       - uses: actions/setup-go@v2

.github/workflows/code-cov.yml

@@ -1,18 +1,20 @@
 on:
   pull_request:
-    paths:
-      - .github/workflows/code-cov.yml
-      - "**/*.go"
-      - "chain/**"
-      - "cmd/**"
-      - "dot/**"
-      - "internal/**"
-      - "lib/**"
-      - "pkg/**"
-      - "tests/**"
-      - .codecov.yml
-      - go.mod
-      - go.sum
+    # Commented paths to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # paths:
+    #   - .github/workflows/code-cov.yml
+    #   - "**/*.go"
+    #   - "chain/**"
+    #   - "cmd/**"
+    #   - "dot/**"
+    #   - "internal/**"
+    #   - "lib/**"
+    #   - "pkg/**"
+    #   - "tests/**"
+    #   - .codecov.yml
+    #   - go.mod
+    #   - go.sum
 name: code-cov
 env:
   GO111MODULE: on

.github/workflows/copyright.yml

@@ -3,10 +3,12 @@ on:
   pull_request:
     branches:
       - development
-    paths:
-      - .github/workflows/copyright.yml
-      - "**/*.go"
-      - "**/*.proto"
+    # Commented paths to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # paths:
+    #   - .github/workflows/copyright.yml
+    #   - "**/*.go"
+    #   - "**/*.proto"
 
 jobs:
   copyright-check:

.github/workflows/docker-grandpa.yml

@@ -1,17 +1,19 @@
 on:
   pull_request:
-    paths:
-      - .github/workflows/docker-grandpa.yml
-      - "**/*.go"
-      - "chain/**"
-      - "cmd/**"
-      - "dot/**"
-      - "internal/**"
-      - "lib/**"
-      - "pkg/**"
-      - "tests/stress/**"
-      - go.mod
-      - go.sum
+    # Commented paths to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # paths:
+    #   - .github/workflows/docker-grandpa.yml
+    #   - "**/*.go"
+    #   - "chain/**"
+    #   - "cmd/**"
+    #   - "dot/**"
+    #   - "internal/**"
+    #   - "lib/**"
+    #   - "pkg/**"
+    #   - "tests/stress/**"
+    #   - go.mod
+    #   - go.sum
 name: docker-grandpa
 env:
   GO111MODULE: on

.github/workflows/docker-js.yml

@@ -1,17 +1,19 @@
 on:
   pull_request:
-    paths:
-      - .github/workflows/docker-js.yml
-      - "**/*.go"
-      - "chain/**"
-      - "cmd/**"
-      - "dot/**"
-      - "internal/**"
-      - "lib/**"
-      - "pkg/**"
-      - "tests/polkadotjs_test/**"
-      - go.mod
-      - go.sum
+    # Commented paths to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # paths:
+    #   - .github/workflows/docker-js.yml
+    #   - "**/*.go"
+    #   - "chain/**"
+    #   - "cmd/**"
+    #   - "dot/**"
+    #   - "internal/**"
+    #   - "lib/**"
+    #   - "pkg/**"
+    #   - "tests/polkadotjs_test/**"
+    #   - go.mod
+    #   - go.sum
 name: docker-js
 env:
   GO111MODULE: on

.github/workflows/docker-rpc.yml

@@ -1,17 +1,19 @@
 on:
   pull_request:
-    paths:
-      - .github/workflows/docker-rpc.yml
-      - "**/*.go"
-      - "chain/**"
-      - "cmd/**"
-      - "dot/**"
-      - "internal/**"
-      - "lib/**"
-      - "pkg/**"
-      - "tests/rpc/**"
-      - go.mod
-      - go.sum
+    # Commented paths to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # paths:
+    #   - .github/workflows/docker-rpc.yml
+    #   - "**/*.go"
+    #   - "chain/**"
+    #   - "cmd/**"
+    #   - "dot/**"
+    #   - "internal/**"
+    #   - "lib/**"
+    #   - "pkg/**"
+    #   - "tests/rpc/**"
+    #   - go.mod
+    #   - go.sum
 name: docker-rpc
 env:
   GO111MODULE: on

.github/workflows/docker-stable.yml

@@ -1,17 +1,19 @@
 on:
   pull_request:
-    paths:
-      - .github/workflows/docker-stable.yml
-      - "**/*.go"
-      - "chain/**"
-      - "cmd/**"
-      - "dot/**"
-      - "internal/**"
-      - "lib/**"
-      - "pkg/**"
-      - scripts/integration-test-all.sh
-      - go.mod
-      - go.sum
+    # Commented paths to avoid skipping required workflow
+    # See https://github.community/t/feature-request-conditional-required-checks/16761
+    # paths:
+    #   - .github/workflows/docker-stable.yml
+    #   - "**/*.go"
+    #   - "chain/**"
+    #   - "cmd/**"
+    #   - "dot/**"
+    #   - "internal/**"
+    #   - "lib/**"
+    #   - "pkg/**"
+    #   - scripts/integration-test-all.sh
+    #   - go.mod
+    #   - go.sum
 name: docker-stable
 env:
   GO111MODULE: on