Commit 8e05e80
![google-labs-jules[bot]](https://avatars.githubusercontent.com/in/842251?v=4&size=40){kind=avatar}
Add unit tests for policy.go (#142)
* feat: Add basic Go unit test setup
This commit introduces the initial setup for Go unit testing.
A sample test file `sourcetool/pkg/policy/policy_test.go` has been created with a placeholder test function.
To run the tests, navigate to the `sourcetool` directory and execute:
`go test ./...`
* test: Add unit test for getPolicyPath
This commit adds a unit test for the `getPolicyPath` function in
`sourcetool/pkg/policy/policy.go`.
The test covers various scenarios, including:
- Valid owner and repository names.
- Empty owner name.
- Empty repository name.
- Empty owner and repository names.
* refactor(tests): Simplify custom message passing to assertion helper
This commit refactors the `assertProtectedBranchEquals` helper function
in `sourcetool/pkg/policy/policy_test.go` and its call sites.
Previously, the helper accepted variadic arguments (`msgAndArgs ...interface{}`)
for custom messages and performed `fmt.Sprintf` internally. This change
simplifies the helper's signature and logic:
1. **Helper Signature Change:**
* `assertProtectedBranchEquals` now accepts a single pre-formatted
`customMessage string` argument instead of variadic arguments.
2. **Call Site Updates:**
* All calls to `assertProtectedBranchEquals` in the test functions
(`TestGetBranchPolicy_Local_SpecificFound`,
`TestGetBranchPolicy_Local_DefaultCases`,
`TestGetBranchPolicy_Remote_SpecificFound`,
`TestGetBranchPolicy_Remote_DefaultCases`) have been updated to
use `fmt.Sprintf` to format the custom message string before
passing it to the helper.
This change makes the helper function's interface cleaner and gives
callers more direct control over the formatting of custom messages,
improving the overall clarity of the test code.
* fix(tests): Correct test failures and improve helper logic
This commit addresses several issues that caused test failures in
`sourcetool/pkg/policy/policy_test.go` and improves the robustness
of the test suite.
Corrections include:
1. **Added Missing Import:**
* Added the `reflect` package import to `pkg/policy/policy_test.go`,
which is required for `reflect.DeepEqual` used in the assertion helper.
2. **Aligned Go-GitHub Version:**
* Updated the import for `github.com/google/go-github` in
`pkg/policy/policy_test.go` from `v50` to `v69` to match the
version used in `pkg/gh_control/connection.go`, resolving
type mismatches.
3. **Refined `assertProtectedBranchEquals` Helper:**
* When `ignoreSince` is `false`, the `Since` fields of the
`ProtectedBranch` structs are now compared explicitly using
`actual.Since.Equal(expected.Since)`.
* For the `reflect.DeepEqual` comparison of the remaining fields,
copies of the structs are made, and their `Since` fields are
zeroed out. This ensures accurate comparison of other struct
members without interference from `time.Time` comparison nuances,
while still allowing precise `Since` validation when needed.
4. **Corrected `expectedPath` in Remote Default Cases:**
* In `TestGetBranchPolicy_Remote_DefaultCases`, the `expectedPath`
for scenarios where a remote policy is successfully fetched but
results in a default branch rule (e.g., branch not found in policy,
empty/nil protected branches) has been changed from the mocked
HTML URL to `"DEFAULT"`. This aligns the test expectation with
the actual behavior of the `getBranchPolicy` function.
After these changes, all tests in `pkg/policy` pass successfully.
* refactor(tests): Remove redundant TestHelloWorld and TestGetPolicyPath
This commit removes two test functions from `sourcetool/pkg/policy/policy_test.go`
that are no longer necessary:
1. **`TestHelloWorld`:** This was an initial placeholder test and served
no ongoing purpose.
2. **`TestGetPolicyPath`:** This function tested the `getPolicyPath` string
formatting utility. The correct behavior of `getPolicyPath` is now
implicitly and sufficiently verified by the `validateMockServerRequestPath`
helper, which is used in all remote policy test functions
(`TestGetBranchPolicy_Remote_*`). The helper constructs the expected
API path using `getPolicyPath` and compares it against the actual
request path received by the mock server.
Removing these functions cleans up the test file by eliminating
obsolete and redundant test logic.
* remove github/v50
Signed-off-by: Tom Hennen <[email protected]>
* Refactor: Consolidate common HTTP test setup in policy_test.go (#3)
I introduced a new helper function, `setupMockGitHubTestEnv`, to encapsulate the common logic for setting up a mock GitHub environment for testing policy fetching. This includes:
- Creating an `httptest.Server` with a specified handler.
- Configuring an `http.Client` and `github.Client` to use the test server.
- Returning a `*gh_control.GitHubConnection` pre-configured for the test environment and the `*httptest.Server` instance.
I refactored the following test functions to use this new helper:
- TestGetBranchPolicy_Remote_SpecificFound
- TestGetBranchPolicy_Remote_DefaultCases
- TestGetBranchPolicy_Remote_ServerError
- TestGetBranchPolicy_Remote_MalformedJSON
This change significantly reduces code duplication and simplifies the setup within these tests, making them cleaner and easier to maintain.
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
* add go.work.sum changes
Signed-off-by: Tom Hennen <[email protected]>
* feat: add unit tests for compute functions in policy.go
This commit introduces unit tests for the following functions in `sourcetool/pkg/policy/policy.go`:
- computeEligibleSlsaLevel
- computeEligibleSince
- computeSlsaLevel
- computeReviewEnforced
- computeImmutableTags
The tests cover various scenarios, including valid inputs, edge cases, and error conditions, ensuring the correctness and robustness of these policy computation functions. The tests are written in a table-driven style, consistent with existing tests in `sourcetool/pkg/policy/policy_test.go`.
* refactor: simplify and clarify tests for policy compute functions
This commit refactors the unit tests for the `compute*` functions in `sourcetool/pkg/policy/policy_test.go`.
The main changes include:
- Consolidating redundant test cases where multiple setups were testing the same underlying logic path. This was particularly applied to:
- `TestComputeEligibleSlsaLevel`: Reduced multiple "Level 1" scenarios based on absent controls into a single case.
- `TestComputeReviewEnforced` and `TestComputeImmutableTags`:
- Combined cases where the policy did not require the control (making control state irrelevant).
- Combined success cases for compliant controls into a single case representing `Policy.Since >= Control.Since`.
- Clarifying test case names to better describe the specific scenario and expected outcome.
- Removing unused test variables that became redundant after consolidation.
These changes make the test suite more concise, readable, and maintainable without sacrificing coverage of distinct logical paths or critical boundary conditions for the tested functions.
* feat: add unit tests for evaluateControls in policy.go
This commit introduces unit tests for the `evaluateControls` function in `sourcetool/pkg/policy/policy.go`.
The tests cover a range of scenarios, including:
- Successful evaluation where all required controls (SLSA level, review enforcement, immutable tags) are met.
- Successful evaluation with various combinations of controls being met or not required by policy.
- Error scenarios where `computeSlsaLevel`, `computeReviewEnforced`, or `computeImmutableTags` (internal calls from `evaluateControls`) return errors.
A table-driven approach was used for the tests, consistent with other tests in the `policy_test.go` file. This ensures comprehensive coverage of the function's logic and error handling.
* Output:
refactor: Simplify policyPath assertion in TestEvaluateProv
Simplifies the assertion logic for the returned `policyPath` in the
`TestEvaluateProv` function in `sourcetool/pkg/policy/policy_test.go`.
The `expectedPolicyPath` values for test cases involving malformed
provenance have been updated to "DEFAULT" when the test setup uses a
policy file that does not explicitly define the branch under test.
This reflects that `getBranchPolicy` determines the path as "DEFAULT"
before `attest.GetProvPred` might return an error due to the
malformed provenance.
The complex conditional logic previously used for asserting the
`policyPath` has been removed and replaced with a direct comparison
against the correctly prepared `actualPolicyPath`.
This change makes the test logic clearer and easier to understand.
All tests continue to pass.
* simplify path handling
Signed-off-by: Tom Hennen <[email protected]>
* Refactor: Further simplify TestEvaluateProv_Success
Removes the t.Run() call from TestEvaluateProv_Success and inlines
simple data variables directly into the test logic.
This change makes the test more concise and easier to read,
following its conversion from a table-driven test to a single-case test.
Complex struct initializations are kept as local variables to maintain
clarity.
* further simplify test case
Signed-off-by: Tom Hennen <[email protected]>
* fix: Align Go version in workflow with go.work
Updates the GitHub Action workflow for Go unit tests to use
Go version 1.23.5, as specified in the `go.work` file.
This ensures consistency between the CI environment and the
project's defined Go version.
---------
Signed-off-by: Tom Hennen <[email protected]>
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>1 parent 0033be9 commit 8e05e80
File tree
6 files changed
+1678
-280
lines changed- .github/workflows
- sourcetool
- pkg
- attest
- policy
6 files changed
+1678
-280
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
6 | 7 | | |
7 | | - | |
| 8 | + | |
8 | 9 | | |
9 | 10 | | |
10 | 11 | | |
11 | | - | |
12 | 12 | | |
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
16 | 16 | | |
17 | | - | |
18 | 17 | | |
19 | 18 | | |
20 | 19 | | |
21 | | - | |
22 | 20 | | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
30 | | - | |
31 | | - | |
32 | 21 | | |
33 | | - | |
34 | 22 | | |
35 | 23 | | |
36 | 24 | | |
37 | 25 | | |
38 | 26 | | |
39 | 27 | | |
| 28 | + | |
40 | 29 | | |
41 | 30 | | |
42 | 31 | | |
| |||
57 | 46 | | |
58 | 47 | | |
59 | 48 | | |
60 | | - | |
61 | 49 | | |
62 | | - | |
63 | 50 | | |
64 | 51 | | |
65 | 52 | | |
66 | 53 | | |
67 | 54 | | |
68 | | - | |
69 | 55 | | |
70 | 56 | | |
71 | 57 | | |
72 | 58 | | |
73 | 59 | | |
74 | 60 | | |
75 | 61 | | |
76 | | - | |
77 | 62 | | |
78 | 63 | | |
79 | | - | |
80 | | - | |
81 | 64 | | |
82 | | - | |
83 | | - | |
84 | 65 | | |
85 | 66 | | |
86 | 67 | | |
87 | | - | |
88 | | - | |
89 | 68 | | |
90 | 69 | | |
91 | 70 | | |
92 | | - | |
93 | | - | |
94 | 71 | | |
95 | 72 | | |
96 | 73 | | |
| |||
106 | 83 | | |
107 | 84 | | |
108 | 85 | | |
109 | | - | |
110 | 86 | | |
111 | 87 | | |
112 | 88 | | |
113 | 89 | | |
114 | | - | |
115 | 90 | | |
116 | 91 | | |
117 | | - | |
| 92 | + | |
118 | 93 | | |
119 | 94 | | |
120 | 95 | | |
| |||
134 | 109 | | |
135 | 110 | | |
136 | 111 | | |
137 | | - | |
138 | 112 | | |
139 | 113 | | |
140 | 114 | | |
141 | 115 | | |
142 | 116 | | |
143 | | - | |
144 | | - | |
145 | 117 | | |
0 commit comments