Add unit tests for policy.go #142
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 ./...`
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.
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.
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.
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.
Signed-off-by: Tom Hennen <tomhennen@google.com>
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>
Signed-off-by: Tom Hennen <tomhennen@google.com>
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`.
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.
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.
Adds test coverage for policy.go
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.
Add tests for EvaluateControl and EvaluateProv
Signed-off-by: Tom Hennen <tomhennen@google.com>
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.
Signed-off-by: Tom Hennen <tomhennen@google.com>
…s-further Refactor simplify evalprovsuccess further
Contributor
Author
|
@puerco will this mess up anything you already have planned? |
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.
fix: Align Go version in workflow with go.work
Collaborator
|
LGTM. It's a lot of tests to check at once but at least it gives the package a decent amount of coverage. I have some suggestions for the CI, etc, but I'll address them in bulk with the other packages. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Most of these tests brought to you by Jules because I wanted to give it a try.
They seem to provide decent coverage and will hopefully help to catch any regressions as we add new features to handle source track updates (like user-provided controls).
I have reviewed them and while it may not be how I'd have done this it doesn't seem too far off. Hopefully they're no worse to maintain than human written tests.