#704 Add more tests to increase test coverage

.github/.testcoverage.yml

@@ -2,5 +2,5 @@
 
 profile: 'cover.out'
 threshold:
-  total: 50
+  total: 90
   file: 0

.github/workflows/run_unit_tests.sh

@@ -0,0 +1,27 @@
+#!/bin/bash
+
+# Runs the Go unit tests with coverage and writes the merged profile to cover.out.
+#
+# The main suite runs with -tags=dev. url_liveness.go gates its HTTP logic behind
+# that dev flag (and its !dev-tagged test is excluded), so it is unreachable in the
+# dev run. We re-run the utils/cli package without the dev tag to collect real
+# coverage for the liveness checks and merge it in, so it shows up in the coverage
+# report. The non-dev package build reports the same canonical file paths as the
+# dev run, so gocovmerge unions the two cleanly.
+
+set -euo pipefail
+
+GOCOVMERGE_VERSION="b5bfa59ec0adc420475f97f89b58045c721d761c"
+
+# shellcheck disable=SC2046
+go test -tags=dev -json -v $(go list ./... | grep -v '/integration_tests') \
+    -coverprofile=./cover.out -covermode=set -coverpkg=./... 2>&1 \
+    | tee /tmp/gotest.log | gotestfmt
+
+go test -json -v -covermode=set -coverprofile=./cover_url_liveness.out \
+    -coverpkg=./utils/cli/... ./utils/cli/ 2>&1 \
+    | tee -a /tmp/gotest.log | gotestfmt
+
+go install "github.com/wadey/gocovmerge@${GOCOVMERGE_VERSION}"
+gocovmerge ./cover.out ./cover_url_liveness.out > ./cover_merged.out
+mv ./cover_merged.out ./cover.out

.github/workflows/verify.yml

@@ -109,11 +109,7 @@ jobs:
             ${{ runner.os }}-go-
       - uses: haveyoudebuggedit/gotestfmt-action@v2
       - name: Run tests
-        run: |
-          set -euo pipefail
-          # shellcheck disable=SC2046
-          go test -tags=dev -json -v $(go list ./... | grep -v '/integration_tests') -coverprofile=./cover.out -covermode=set -coverpkg=./... 2>&1 | tee /tmp/gotest.log | gotestfmt
-          go test -v utils/cli/url_liveness.go  utils/cli/url_liveness_test.go
+        run: .github/workflows/run_unit_tests.sh
         env:
           GH_TOKEN: ${{ secrets.GH_TOKEN }}
           GOCOVER_MODE: set
@@ -211,7 +207,6 @@ jobs:
         run: .github/workflows/filter_coverage.sh
       - name: Report test coverage
         uses: vladopajic/go-test-coverage@v2
-        continue-on-error: true
         with:
           config: ./.github/.testcoverage.yml
           git-token: ${{ github.ref_name == 'master' && secrets.GITHUB_TOKEN || '' }}

go.mod

@@ -62,6 +62,7 @@ require (
 	github.com/cespare/xxhash v1.1.0 // indirect
 	github.com/cespare/xxhash/v2 v2.2.0 // indirect
 	github.com/chzyer/readline v1.5.1 // indirect
+	github.com/cockroachdb/apd/v2 v2.0.2 // indirect
 	github.com/cockroachdb/errors v1.10.0 // indirect
 	github.com/cockroachdb/logtags v0.0.0-20230118201751-21c54148d20b // indirect
 	github.com/cockroachdb/redact v1.1.5 // indirect

testutil/sample/sample.go

@@ -12,3 +12,11 @@ func AccAddress() string {
 
 	return sdk.AccAddress(addr).String()
 }
+
+// ValAddress returns a sample validator operator (valoper) address.
+func ValAddress() string {
+	pk := ed25519.GenPrivKey().PubKey()
+	addr := pk.Address()
+
+	return sdk.ValAddress(addr).String()
+}

types/pki/errors.go

@@ -55,6 +55,8 @@ var (
 	ErrCRLSignerCertificateInvalidVersion                = errors.Register(ModuleName, 443, "invalid CRLSignerCertificate version")
 	ErrCertVidNotEqualToIssuerVid                        = errors.Register(ModuleName, 444, "certificate's vid is not equal to issuer's vid")
 	ErrCertPidNotEqualToIssuerPid                        = errors.Register(ModuleName, 445, "certificate's pid is not equal to issuer's pid")
+	ErrUnauthorizedCertIssuer                            = errors.Register(ModuleName, 446, "unauthorized certificate issuer")
+	ErrVVSCChainVerificationFailed                       = errors.Register(ModuleName, 447, "VVSC chain verification failed")
 )
 
 func NewErrUnauthorizedRole(transactionName string, requiredRole types.AccountRole) error {
@@ -152,6 +154,12 @@ func NewErrInvalidCertificate(e interface{}) error {
 		e)
 }
 
+func NewErrVVSCChainVerificationFailed(subject string, subjectKeyID string) error {
+	return errors.Wrapf(ErrVVSCChainVerificationFailed,
+		"VVSC chain verification failed for subject=%v subjectKeyID=%v",
+		subject, subjectKeyID)
+}
+
 func NewErrInvalidDataDigestType(dataDigestType uint32, allowedDataDigestTypes []uint32) error {
 	return errors.Wrapf(ErrInvalidDataDigestType,
 		"Invalid DataDigestType: %d. Supported types are: %v", dataDigestType, allowedDataDigestTypes)
@@ -192,7 +200,7 @@ func NewErrNonRootCertificateSelfSigned() error {
 }
 
 func NewErrUnauthorizedCertIssuer(subject string, subjectKeyID string) error {
-	return errors.Wrapf(sdkerrors.ErrUnauthorized,
+	return errors.Wrapf(ErrUnauthorizedCertIssuer,
 		"Issuer and authorityKeyID of new certificate with subject=%v and subjectKeyID=%v "+
 			"must be the same as ones of existing certificates with the same subject and subjectKeyID",
 		subject, subjectKeyID)

utils/cli/url_liveness_test.go

@@ -35,14 +35,23 @@ func TestIsLiveURL(t *testing.T) {
 		want       bool
 	}{
 		{"200 OK", http.StatusOK, true},
+		{"201 created", http.StatusCreated, true},
+		{"204 no content", http.StatusNoContent, true},
+		{"299 highest 2xx", 299, true},
 		{"301 redirect", http.StatusMovedPermanently, true},
+		{"302 found", http.StatusFound, true},
+		{"308 permanent redirect", http.StatusPermanentRedirect, true},
+		{"399 highest 3xx", 399, true},
 		{"401 unauthorized", http.StatusUnauthorized, true},
 		{"403 forbidden", http.StatusForbidden, true},
 		{"405 method not allowed", http.StatusMethodNotAllowed, true},
 		{"451 unavailable for legal reasons", http.StatusUnavailableForLegalReasons, true},
+		{"400 bad request", http.StatusBadRequest, false},
 		{"404 not found", http.StatusNotFound, false},
+		{"429 too many requests", http.StatusTooManyRequests, false},
 		{"500 internal server error", http.StatusInternalServerError, false},
 		{"502 bad gateway", http.StatusBadGateway, false},
+		{"503 service unavailable", http.StatusServiceUnavailable, false},
 	}
 
 	for _, tt := range tests {
@@ -68,6 +77,38 @@ func TestIsLiveURLUnreachable(t *testing.T) {
 	require.False(t, IsLiveURL(u.String()))
 }
 
+// TestIsLiveURLRequestCreationError covers the path where the HTTP request
+// cannot even be constructed (e.g. the URL is malformed), so no network call
+// is attempted and the URL is treated as not live.
+func TestIsLiveURLRequestCreationError(t *testing.T) {
+	tests := []struct {
+		name string
+		url  string
+	}{
+		{"invalid control character", "http://example.com/\x7f"},
+		{"missing protocol scheme", ":no-scheme"},
+		{"control character in host", "http://exa\x00mple.com"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.False(t, IsLiveURL(tt.url))
+		})
+	}
+}
+
+// TestIsLiveURLAllowed4XXNotMutated guards against the allow-list of 4XX status
+// codes being changed unexpectedly, since IsLiveURL relies on it for the
+// allowed-status path.
+func TestIsLiveURLAllowed4XXNotMutated(t *testing.T) {
+	require.ElementsMatch(t, []int{
+		http.StatusUnauthorized,
+		http.StatusForbidden,
+		http.StatusUnavailableForLegalReasons,
+		http.StatusMethodNotAllowed,
+	}, allowed4XXStatusCodes)
+}
+
 func TestCheckURLsForLiveness(t *testing.T) {
 	okSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)

utils/cli/utils_test.go

@@ -0,0 +1,75 @@
+// Copyright 2020 DSR Corporation
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package cli
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestReadFromFileExistingFile(t *testing.T) {
+	const content = "certificate-content\nmultiple-lines\n"
+
+	path := filepath.Join(t.TempDir(), "cert.pem")
+	require.NoError(t, os.WriteFile(path, []byte(content), 0o600))
+
+	got, err := ReadFromFile(path)
+	require.NoError(t, err)
+	require.Equal(t, content, got)
+}
+
+func TestReadFromFileEmptyFile(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "empty.pem")
+	require.NoError(t, os.WriteFile(path, []byte{}, 0o600))
+
+	got, err := ReadFromFile(path)
+	require.NoError(t, err)
+	require.Equal(t, "", got)
+}
+
+// TestReadFromFileNotAPath covers the branch where the target does not exist on
+// disk and is therefore returned verbatim (e.g. a PEM string passed inline).
+func TestReadFromFileNotAPath(t *testing.T) {
+	tests := []struct {
+		name   string
+		target string
+	}{
+		{"inline pem", "-----BEGIN CERTIFICATE-----\nMIIB...\n-----END CERTIFICATE-----"},
+		{"plain string", "not-a-file"},
+		{"empty string", ""},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got, err := ReadFromFile(tt.target)
+			require.NoError(t, err)
+			require.Equal(t, tt.target, got)
+		})
+	}
+}
+
+// TestReadFromFileReadError covers the branch where os.Stat succeeds but the
+// subsequent read fails. Pointing at a directory triggers a read error while
+// still passing the existence check.
+func TestReadFromFileReadError(t *testing.T) {
+	dir := t.TempDir()
+
+	got, err := ReadFromFile(dir)
+	require.Error(t, err)
+	require.Equal(t, "", got)
+}

x/compliance/handler_provision_model_test.go

@@ -6,6 +6,7 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 	testconstants "github.com/zigbee-alliance/distributed-compliance-ledger/integration_tests/constants"
 	"github.com/zigbee-alliance/distributed-compliance-ledger/x/compliance/types"
@@ -198,3 +199,43 @@ func TestHandler_SchemaVersion_ProvisionModel_StampsCurrentOnCreate(t *testing.T
 	provisionalModel, _ := queryProvisionalModel(setup, vid, pid, softwareVersion, certificationType)
 	require.Equal(t, provisionalModel.CurrentSchemaVersion(), provisionalModel.SchemaVersion)
 }
+
+// Covers the default switch arm: existing compliance info whose status is none of
+// provisional/certified/revoked.
+func TestHandler_ProvisionModel_ExistingInfoUnknownStatus(t *testing.T) {
+	setup, vid, pid, softwareVersion, softwareVersionString, certificationType := setupProvisionModel(t)
+
+	setup.Keeper.SetComplianceInfo(setup.Ctx, &types.ComplianceInfo{
+		Vid:                                vid,
+		Pid:                                pid,
+		SoftwareVersion:                    softwareVersion,
+		CertificationType:                  certificationType,
+		SoftwareVersionCertificationStatus: 99, // not provisional/certified/revoked
+	})
+
+	_, err := setup.provisionModel(vid, pid, softwareVersion, softwareVersionString, certificationType, setup.CertificationCenter)
+	require.ErrorIs(t, err, types.ErrComplianceInfoAlreadyExist)
+}
+
+// Covers the branch where the stored model version's software version string does
+// not match the one in the message.
+func TestHandler_ProvisionModel_ModelVersionStringMismatch(t *testing.T) {
+	setup, vid, pid, softwareVersion, _, certificationType := setupProvisionModel(t)
+
+	_, err := setup.provisionModel(vid, pid, softwareVersion, "different-version-string", certificationType, setup.CertificationCenter)
+	require.ErrorIs(t, err, types.ErrModelVersionStringDoesNotMatch)
+}
+
+// Covers the branch where the stored model version's CD version number does not
+// match the one in the message (software version string still matches).
+func TestHandler_ProvisionModel_CdVersionNumberMismatch(t *testing.T) {
+	setup := setup(t)
+	vid, pid, softwareVersion := testconstants.Vid, testconstants.Pid, testconstants.SoftwareVersion
+
+	modelVersion := newModelVersion(vid, pid, softwareVersion, testconstants.SoftwareVersionString)
+	modelVersion.CdVersionNumber = testconstants.CdVersionNumber + 1
+	setup.ModelKeeper.On("GetModelVersion", mock.Anything, vid, pid, softwareVersion).Return(*modelVersion, true)
+
+	_, err := setup.provisionModel(vid, pid, softwareVersion, testconstants.SoftwareVersionString, types.ZigbeeCertificationType, setup.CertificationCenter)
+	require.ErrorIs(t, err, types.ErrModelVersionStringDoesNotMatch)
+}

x/compliance/keeper/device_software_compliance_test.go

@@ -1,6 +1,5 @@
 package keeper_test
 
-/*
 import (
 	"strconv"
 	"testing"
@@ -13,53 +12,45 @@ import (
 	"github.com/zigbee-alliance/distributed-compliance-ledger/x/compliance/types"
 )
 
-// Prevent strconv unused error
-var _ = strconv.IntSize
-
 func createNDeviceSoftwareCompliance(keeper *keeper.Keeper, ctx sdk.Context, n int) []types.DeviceSoftwareCompliance {
 	items := make([]types.DeviceSoftwareCompliance, n)
 	for i := range items {
 		items[i].CDCertificateId = strconv.Itoa(i)
 
 		keeper.SetDeviceSoftwareCompliance(ctx, &items[i])
 	}
+
 	return items
 }
 
 func TestDeviceSoftwareComplianceGet(t *testing.T) {
-	keeper, ctx := keepertest.ComplianceKeeper(t)
+	keeper, ctx := keepertest.ComplianceKeeper(t, nil, nil)
 	items := createNDeviceSoftwareCompliance(keeper, ctx, 10)
 	for _, item := range items {
-		rst, found := keeper.GetDeviceSoftwareCompliance(ctx,
-			item.CDCertificateId,
-		)
+		rst, found := keeper.GetDeviceSoftwareCompliance(ctx, item.CDCertificateId)
 		require.True(t, found)
 		require.Equal(t,
 			nullify.Fill(&item),
 			nullify.Fill(&rst),
 		)
 	}
 }
+
 func TestDeviceSoftwareComplianceRemove(t *testing.T) {
-	keeper, ctx := keepertest.ComplianceKeeper(t)
+	keeper, ctx := keepertest.ComplianceKeeper(t, nil, nil)
 	items := createNDeviceSoftwareCompliance(keeper, ctx, 10)
 	for _, item := range items {
-		keeper.RemoveDeviceSoftwareCompliance(ctx,
-			item.CdCertificateId,
-		)
-		_, found := keeper.GetDeviceSoftwareCompliance(ctx,
-			item.CdCertificateId,
-		)
+		keeper.RemoveDeviceSoftwareCompliance(ctx, item.CDCertificateId)
+		_, found := keeper.GetDeviceSoftwareCompliance(ctx, item.CDCertificateId)
 		require.False(t, found)
 	}
 }
 
 func TestDeviceSoftwareComplianceGetAll(t *testing.T) {
-	keeper, ctx := keepertest.ComplianceKeeper(t)
+	keeper, ctx := keepertest.ComplianceKeeper(t, nil, nil)
 	items := createNDeviceSoftwareCompliance(keeper, ctx, 10)
 	require.ElementsMatch(t,
 		nullify.Fill(items),
 		nullify.Fill(keeper.GetAllDeviceSoftwareCompliance(ctx)),
 	)
 }
-*/