diff --git a/Makefile b/Makefile index abb955e8..df62cac3 100644 --- a/Makefile +++ b/Makefile @@ -29,6 +29,13 @@ GOLANGCI_LINT_VERSION=latest BUILD_DIR := build PLATFORM?=ocp +GO=go +TEST_OPTIONS?=-timeout=20m +# Packages that actually have test files, excluding vendored code and the +# root package (github.com/ComplianceAsCode/ocp4e2e), whose *_test.go files are +# the cluster-backed e2e suite and cannot run without a live OpenShift cluster. +TESTABLE_PKGS=$(shell $(GO) list -f '{{if or .TestGoFiles .XTestGoFiles}}{{.ImportPath}}{{end}}' ./... | grep -v -E '/vendor/' | grep -v -E '^github.com/ComplianceAsCode/ocp4e2e$$') + .PHONY: all all: e2e @@ -60,6 +67,10 @@ help: ## Show this help screen @awk 'BEGIN {FS = ":.*##"; printf "\nUsage:\n make \033[36m\033[0m\n"} /^[a-zA-Z0-9_-]+:.*?##/ { printf " \033[36m%-15s\033[0m %s\n", $$1, $$2 } /^##@/ { printf "\n\033[1m%s\033[0m\n", substr($$0, 5) } ' $(MAKEFILE_LIST) +.PHONY: test-unit +test-unit: ## Run the unit tests (no cluster required) + $(GO) test $(TEST_OPTIONS) -cover $(TESTABLE_PKGS) + .PHONY: verify verify: verify-go-lint ## Run all verification targets diff --git a/helpers/utilities.go b/helpers/utilities.go index cbb975e9..12a02f59 100644 --- a/helpers/utilities.go +++ b/helpers/utilities.go @@ -1002,9 +1002,9 @@ func GenerateAssertionFileFromResults( assertions := &RuleTestResults{ RuleResults: make(map[string]RuleTest), } - ruleTest := RuleTest{} afterRemediation := finalResults != nil for ruleName, initialResult := range initialResults { + ruleTest := RuleTest{} ruleTest.DefaultResult = initialResult if afterRemediation { diff --git a/helpers/utilities_test.go b/helpers/utilities_test.go new file mode 100644 index 00000000..bfc6a821 --- /dev/null +++ b/helpers/utilities_test.go @@ -0,0 +1,86 @@ +package helpers + +import ( + "os" + "path" + "testing" + + testConfig "github.com/ComplianceAsCode/ocp4e2e/config" + "gopkg.in/yaml.v2" +) + +// TestGenerateAssertionFileFromResultsNoCarryover is a regression test for +// CMP-4630: GenerateAssertionFileFromResults used to reuse a single RuleTest +// struct across loop iterations, so a rule whose result did not change between +// the initial and final scans inherited the previous rule's +// ResultAfterRemediation value. Because Go map iteration order is randomized, +// the corruption was nondeterministic. +// +// The assertion here holds only when a fresh RuleTest is used per rule: +// - a rule whose result did NOT change must have no result_after_remediation +// - a rule whose result DID change must record exactly its final result +func TestGenerateAssertionFileFromResultsNoCarryover(t *testing.T) { + logDir := t.TempDir() + tc := &testConfig.TestConfig{LogDir: logDir} + + // Interleave changed and unchanged rules. Many rules increase the chance + // the old (buggy) code carries a stale value into an unchanged rule + // regardless of the randomized map iteration order. + initial := map[string]string{} + final := map[string]string{} + // Changed rules: FAIL -> PASS + changed := []string{"rule-changed-01", "rule-changed-02", "rule-changed-03", "rule-changed-04"} + for _, r := range changed { + initial[r] = "FAIL" + final[r] = "PASS" + } + // Unchanged rules: PASS -> PASS and FAIL -> FAIL + unchangedPass := []string{"rule-pass-01", "rule-pass-02", "rule-pass-03", "rule-pass-04", "rule-pass-05"} + for _, r := range unchangedPass { + initial[r] = "PASS" + final[r] = "PASS" + } + unchangedFail := []string{"rule-fail-01", "rule-fail-02", "rule-fail-03", "rule-fail-04", "rule-fail-05"} + for _, r := range unchangedFail { + initial[r] = "FAIL" + final[r] = "FAIL" + } + + const fileName = "assertions.yml" + if err := GenerateAssertionFileFromResults(tc, nil, fileName, initial, final); err != nil { + t.Fatalf("GenerateAssertionFileFromResults returned error: %v", err) + } + + data, err := os.ReadFile(path.Join(logDir, fileName)) + if err != nil { + t.Fatalf("failed to read generated assertion file: %v", err) + } + var got RuleTestResults + if err := yaml.Unmarshal(data, &got); err != nil { + t.Fatalf("failed to unmarshal assertion file: %v", err) + } + + if len(got.RuleResults) != len(initial) { + t.Fatalf("expected %d rules, got %d", len(initial), len(got.RuleResults)) + } + + for rule, rt := range got.RuleResults { + wantDefault := initial[rule] + if rt.DefaultResult != wantDefault { + t.Errorf("rule %s: default_result = %v, want %v", rule, rt.DefaultResult, wantDefault) + } + if final[rule] == initial[rule] { + // Unchanged rule: must NOT carry a result_after_remediation. + if rt.ResultAfterRemediation != nil { + t.Errorf("rule %s: unchanged result but result_after_remediation = %v (carryover bug)", + rule, rt.ResultAfterRemediation) + } + } else { + // Changed rule: must record its own final result. + if rt.ResultAfterRemediation != final[rule] { + t.Errorf("rule %s: result_after_remediation = %v, want %v", + rule, rt.ResultAfterRemediation, final[rule]) + } + } + } +}