test: add unit tests for resolveBuilder in cmd/ci#3570
test: add unit tests for resolveBuilder in cmd/ci#3570Ankitsinghsisodya wants to merge 2 commits intoknative:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Ankitsinghsisodya The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @Ankitsinghsisodya. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
There was a problem hiding this comment.
Pull request overview
Adds unit tests for cmd/ci/config.go’s CIConfig accessors and the resolveBuilder helper to improve coverage without requiring viper setup, mocks, or external dependencies.
Changes:
- Introduces
cmd/ci/config_test.gowith tests for CIConfig getter/path helpers. - Adds a table-driven test covering
resolveBuilderruntime→builder selection and error behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add tests for CIConfig accessor methods and resolveBuilder helper in cmd/ci/config.go. Covered: - FnGitHubWorkflowFilepath correctly joins fnRoot, workflow dir and filename - OutputPath uses the default dir/filename constants - Verbose, FnRuntime, FnBuilder simple getters - resolveBuilder — all supported runtimes (go, node, typescript, rust, quarkus, springboot, python) both local and remote modes, plus error for unknown runtime - All remaining bool accessors: RegistryLogin, SelfHostedRunner, RemoteBuild, WorkflowDispatch, TestStep, Force - All string accessors: Branch, WorkflowName, KubeconfigSecret, RegistryLoginUrlVar, RegistryUserVar, RegistryPassSecret, RegistryUrlVar Fixes zero-coverage on config.go:221, 226, 282, 286, 290.
556ff32 to
c119d3c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3570 +/- ##
==========================================
+ Coverage 55.52% 56.28% +0.75%
==========================================
Files 180 180
Lines 20465 20465
==========================================
+ Hits 11363 11518 +155
+ Misses 7902 7749 -153
+ Partials 1200 1198 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Removed accessor tests for one-liner getters (Verbose, FnRuntime, FnBuilder, BoolAccessors, StringAccessors, OutputPath, FnGitHubWorkflowFilepath) — they test Go's struct assignment, not any real logic. Kept TestResolveBuilder (15 sub-cases) which covers the non-trivial branching in resolveBuilder() across all runtimes × local/remote.
test: add unit tests for resolveBuilder in cmd/ci
Related Issue
Closes: #3568
What this PR does
Adds
cmd/ci/config_test.go— a new test file forcmd/ci/config.go.The unexported
resolveBuilderfunction had zero test coverage. Itcontains non-trivial branching logic that selects the correct build strategy
(
host,pack, ors2i) for each runtime × local/remote combination.This PR adds a focused, 15-subcase table-driven test for
resolveBuilderusing struct literals — no viper setup, no mocks, no cluster, no network.
Trivial one-liner getter/accessor methods are intentionally not tested here,
as they add no meaningful coverage and only verify that Go's struct field
assignment works.
Changes
cmd/ci/config_test.goTests added
How to verify
go test ./cmd/ci/... -v -run TestResolveBuilderExpected output: all 15 sub-tests PASS,
ok knative.dev/func/cmd/ci.Checklist
make testpassesmake checkpasses