Commonize OIDC handler test cases with table-driven generation#111
Commonize OIDC handler test cases with table-driven generation#111
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors oidc_handling_test.go to generate OIDC handler URL/auth/log assertion cases via shared, table-driven helpers, reducing repetitive per-ecosystem/per-provider boilerplate while aiming to keep the same coverage.
Changes:
- Adds
oidcProviderFields+buildEcosystemCaseshelpers and supporting structs to generate OIDC test cases across providers. - Rewrites
TestOIDCURLsAreAuthenticatedto use provider variants per ecosystem (including special-case overrides like NuGet mocks and Python/simplebehavior). - Normalizes provider token-exchange endpoint mocks within the shared test runner.
Show a summary per file
| File | Description |
|---|---|
| internal/handlers/oidc_handling_test.go | Replaces many hand-written OIDC URL/auth test cases with helper-generated, table-driven cases and provider-specific token-exchange mocks. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 2
Replace 65 hand-written test cases (13 ecosystems × 5 providers) with table-driven generation using buildEcosystemCases helper. Each ecosystem defines its variants compactly — provider credential fields are shared via oidcProviderFields, and expected log lines / auth URLs are derived automatically with explicit overrides for special cases. Reduces oidc_handling_test.go from 1846 to 685 lines (~63%) while preserving identical test coverage: same 65 test cases, same test names, same assertions, same provider mock endpoints. Special cases handled explicitly: - Docker/Helm JFrog: host-derived log targets - Go proxy/Terraform Azure: host-based credentials - Python GCP: index-url with /simple suffix stripping - RubyGems: dual url+host credential fields - NuGet: service index discovery mocks and resource URL registration - Composer JFrog: dual registry+url fields Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
kbukum1
force-pushed
the
commonize-oidc-handler-tests
branch
from
0950347 to
a12df54
Compare
| nugetMocks := func(baseURL string) []mockHttpRequest { | ||
| host := strings.TrimPrefix(strings.TrimPrefix(baseURL, "https://"), "http://") | ||
| host = strings.SplitN(host, "/", 2)[0] | ||
| pathPrefix := strings.TrimPrefix(baseURL, "https://"+host) | ||
| pathPrefix = strings.TrimSuffix(pathPrefix, "/index.json") | ||
| return []mockHttpRequest{{ | ||
| verb: "GET", | ||
| url: baseURL, | ||
| response: fmt.Sprintf(`{"version":"3.0.0","resources":[{"@id":"https://%s%s/v3/packages","@type":"PackageBaseAddress/3.0.0"}]}`, | ||
| host, pathPrefix), | ||
| }} |
There was a problem hiding this comment.
In the NuGet test case generator, nugetMocks always appends /v3/packages to the derived pathPrefix. For a base service index like https://cloudsmith.example.com/v3/index.json, pathPrefix becomes /v3, producing a mocked resource URL of .../v3/v3/packages (and the test then authenticates that URL). This changes the covered behavior vs the prior hand-written tests (which used .../v3/packages) and doesn’t match typical NuGet service index layouts. Consider special-casing the /v3 prefix (or deriving the resource URL from the index directory) so .../v3/index.json yields .../v3/packages while nested repos (e.g. .../my-repo/index.json) still yield .../my-repo/v3/packages.
Commonize OIDC handler test cases with table-driven generation
Follow-up from #108 — reduces repetitive test code while preserving identical coverage.
Problem
oidc_handling_test.gohad 65 hand-written test cases (13 ecosystems × 5 OIDC providers) with near-identical structure. Adding a new provider required copying ~13 blocks; adding a new ecosystem required copying ~5 blocks. Each block repeated the same provider credential fields, handler factory, and assertion logic.Approach
Introduce
buildEcosystemCaseshelper andoidcProviderFieldsshared function:oidcProviderFields(provider)— returns provider-specific credential fields (aws-region, tenant-id, etc.) shared across all ecosystemsbuildEcosystemCases(name, factory, logLabel, variants)— generates test cases by merging ecosystem URL config with provider fields, deriving log lines and auth URLs automaticallySpecial cases preserved with explicit overrides:
index-urlwith/simplesuffix strippingurl+hostcredential fieldsregistry+urlfieldsResult
oidcProviderFields+ 1 line per ecosystembuildEcosystemCasescall with 5 variants