chore: constructor - update LocalBlob YAML fixtures to UpperCamelCase [2/4]#2324
Conversation
Aligns with open-component-model#962 UpperCamelCase convention. LocalBlob is now the primary type for marshaling; localBlob preserved as legacy alias for backward compatibility. Both descriptor/v2 and descriptor/runtime updated. Adds scheme resolution tests for all 4 aliases (versioned/unversioned, primary/legacy), updates marshal/unmarshal assertions, and adds legacy backward-compat test coverage. Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
✅ Deploy Preview for ocm-website ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThis PR standardizes the local blob access type identifier from lowercase camel case ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Only LocalBlob/v1 (UpperCamelCase) should be the non-deprecated canonical form. All legacy variants — localBlob/v1, LocalBlob, localBlob — are now marked deprecated in the JSON schemas and generator markers, aligning with the intent of ocm-project#962. Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
There was a problem hiding this comment.
🧹 Nitpick comments (3)
bindings/go/descriptor/v2/scheme_test.go (1)
12-42: LGTM — good coverage across canonical/legacy and versioned/unversioned forms.The four tests concisely validate all four permutations of
Scheme.NewObjectresolution forLocalBlob. Assertions userequire.IsTypeon the concrete pointer, which is the right check here.Optional: these tests are structurally identical and could be collapsed into a single table-driven test keyed by
runtime.Type→ expected type, but the current form reads fine and is easy to diff with the alias-registration changes inscheme.go.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/go/descriptor/v2/scheme_test.go` around lines 12 - 42, The tests already cover all permutations (versioned/legacy/unversioned) for LocalBlob resolution using descriptorv2.Scheme.NewObject and the runtime.NewVersionedType/runtime.NewUnversionedType helpers; no changes required—leave TestScheme_ResolvesUpperCamelCase_LocalBlob, TestScheme_ResolvesLegacy_LocalBlob, TestScheme_ResolvesUnversioned_LocalBlob, and TestScheme_ResolvesUnversionedLegacy_LocalBlob as-is (optionally you may collapse them into a single table-driven test keyed by runtime.Type mapping to the expected *descriptorv2.LocalBlob, but that's not required).bindings/go/descriptor/v2/local_access_test.go (2)
46-98: Consider table-driving the two near-identical unmarshal tests.
TestLocalBlob_UnmarshalJSON_LegacyTypeandTestLocalBlob_UnmarshalJSON_UpperCamelCasediffer only in the inputtypestring and the expectedblob.Type.Name. A small table-driven test would remove ~40 lines of duplication and make it trivial to add future aliases. Non-blocking — current form is clear.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/go/descriptor/v2/local_access_test.go` around lines 46 - 98, The two near-duplicate tests TestLocalBlob_UnmarshalJSON_LegacyType and TestLocalBlob_UnmarshalJSON_UpperCamelCase should be combined into a small table-driven test: create a slice of cases with fields like name, inputType (the "type" string used in jsonData), and expectedTypeName; for each case use t.Run(case.name, func(t *testing.T) { build jsonData with case.inputType, unmarshal into a descriptorv2.LocalBlob, then run the same assertions (require.NoError, assert.Equal for blob.Type.Name == case.expectedTypeName, blob.Type.Version == descriptorv2.LocalBlobAccessTypeVersion, LocalReference, MediaType, ReferenceName, and GlobalAccess checks) }); keep using the same variable names (blob, descriptorv2.LocalBlob) and reuse existing constants like descriptorv2.LocalBlobAccessTypeVersion and descriptorv2.LegacyLocalBlobAccessType in the case table to cover aliases.
91-91: Use theLocalBlobAccessTypeconstant instead of the hardcoded"LocalBlob"literal.Elsewhere in this file (e.g., Line 64, Line 113) the test asserts against
descriptorv2.LegacyLocalBlobAccessTyperather than a string literal. For symmetry and to keep the test resilient to future constant renames, use the constant here too.♻️ Proposed change
- assert.Equal(t, "LocalBlob", blob.Type.Name) + assert.Equal(t, descriptorv2.LocalBlobAccessType, blob.Type.Name)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bindings/go/descriptor/v2/local_access_test.go` at line 91, Replace the hardcoded string literal "LocalBlob" in the assertion on blob.Type.Name with the package constant descriptorv2.LocalBlobAccessType (i.e., change assert.Equal(t, "LocalBlob", blob.Type.Name) to assert.Equal(t, descriptorv2.LocalBlobAccessType, blob.Type.Name)) so the test uses the canonical constant (consistent with other assertions that use descriptorv2.LegacyLocalBlobAccessType).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@bindings/go/descriptor/v2/local_access_test.go`:
- Around line 46-98: The two near-duplicate tests
TestLocalBlob_UnmarshalJSON_LegacyType and
TestLocalBlob_UnmarshalJSON_UpperCamelCase should be combined into a small
table-driven test: create a slice of cases with fields like name, inputType (the
"type" string used in jsonData), and expectedTypeName; for each case use
t.Run(case.name, func(t *testing.T) { build jsonData with case.inputType,
unmarshal into a descriptorv2.LocalBlob, then run the same assertions
(require.NoError, assert.Equal for blob.Type.Name == case.expectedTypeName,
blob.Type.Version == descriptorv2.LocalBlobAccessTypeVersion, LocalReference,
MediaType, ReferenceName, and GlobalAccess checks) }); keep using the same
variable names (blob, descriptorv2.LocalBlob) and reuse existing constants like
descriptorv2.LocalBlobAccessTypeVersion and
descriptorv2.LegacyLocalBlobAccessType in the case table to cover aliases.
- Line 91: Replace the hardcoded string literal "LocalBlob" in the assertion on
blob.Type.Name with the package constant descriptorv2.LocalBlobAccessType (i.e.,
change assert.Equal(t, "LocalBlob", blob.Type.Name) to assert.Equal(t,
descriptorv2.LocalBlobAccessType, blob.Type.Name)) so the test uses the
canonical constant (consistent with other assertions that use
descriptorv2.LegacyLocalBlobAccessType).
In `@bindings/go/descriptor/v2/scheme_test.go`:
- Around line 12-42: The tests already cover all permutations
(versioned/legacy/unversioned) for LocalBlob resolution using
descriptorv2.Scheme.NewObject and the
runtime.NewVersionedType/runtime.NewUnversionedType helpers; no changes
required—leave TestScheme_ResolvesUpperCamelCase_LocalBlob,
TestScheme_ResolvesLegacy_LocalBlob, TestScheme_ResolvesUnversioned_LocalBlob,
and TestScheme_ResolvesUnversionedLegacy_LocalBlob as-is (optionally you may
collapse them into a single table-driven test keyed by runtime.Type mapping to
the expected *descriptorv2.LocalBlob, but that's not required).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 48bfe57b-8cc3-4d00-ac38-5a3f3070d67d
📒 Files selected for processing (10)
bindings/go/constructor/construct_resource_test.gobindings/go/constructor/construct_source_test.gobindings/go/descriptor/runtime/local_access.gobindings/go/descriptor/runtime/schemas/LocalBlob.schema.jsonbindings/go/descriptor/v2/helpers_test.gobindings/go/descriptor/v2/local_access.gobindings/go/descriptor/v2/local_access_test.gobindings/go/descriptor/v2/schemas/LocalBlob.schema.jsonbindings/go/descriptor/v2/scheme.gobindings/go/descriptor/v2/scheme_test.go
… [2/4] (open-component-model#2324) ## Summary Updates YAML test fixtures in `bindings/go/constructor/` to use `LocalBlob` (UpperCamelCase) instead of `localBlob`. Part of the localBlob → LocalBlob UpperCamelCase migration for ocm-project/ocm-project#962. ## Changes - `construct_resource_test.go`: YAML fixture `type: localBlob` → `type: LocalBlob` - `construct_source_test.go`: YAML fixture `type: localBlob` → `type: LocalBlob` ## Dependencies Depends on open-component-model#2323 being merged and published (descriptor/v2 with `LocalBlob` as primary type). ## Related PRs - open-component-model#2323 — PR 1: descriptor/v2 + descriptor/runtime (constants, scheme, tests) - **This PR** — PR 2: constructor (YAML fixtures) - open-component-model#2325 — PR 3: cli (test assertions) - open-component-model#2326 — PR 4: kubernetes/controller (test JSON) Resolves part of ocm-project/ocm-project#962 --------- Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com> Co-authored-by: Gerald Morrison (SAP) <gerald.morrison@sap.com> e25a69c
…Case [4/4] (#2326) ## Summary Updates test JSON strings and hardcoded type references in `kubernetes/controller/` to use `LocalBlob/v1` (UpperCamelCase) instead of `localBlob/v1`. Part of the localBlob → LocalBlob UpperCamelCase migration for ocm-project/ocm-project#962. ## Changes - `deployer_controller_test.go`: 9× `{"type":"localBlob/v1"}` → `{"type":"LocalBlob/v1"}` - `deployer_controller_rgd_test.go`: 4× same replacement - `cel/functions/oci_test.go`: replaced hardcoded `runtime.NewVersionedType("localBlob", "v1")` with `runtime.NewVersionedType(v2.LocalBlobAccessType, v2.LocalBlobAccessTypeVersion)` using the exported constant ## Dependencies Depends on #2323 being merged and published (descriptor/v2 with `LocalBlob` as primary type). ## Related PRs - #2323 — PR 1: descriptor/v2 + descriptor/runtime (constants, scheme, tests) - #2324 — PR 2: constructor (YAML fixtures) - #2325 — PR 3: cli (test assertions) - **This PR** — PR 4: kubernetes/controller (test JSON) Resolves part of ocm-project/ocm-project#962 --------- Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com> Co-authored-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
…2325) ## Summary Updates test assertion strings in `cli/cmd/cmd_test.go` from `"localBlob/v1"` to `"LocalBlob/v1"` to match the new primary UpperCamelCase type name. Part of the localBlob → LocalBlob UpperCamelCase migration for ocm-project/ocm-project#962. ## Changes - `cli/cmd/cmd_test.go`: 5 assertion strings updated (`"localBlob/v1"` → `"LocalBlob/v1"`) - Line 829: `r.Equal` assertion - Line 1240: YAML output string - Line 1353: JSON map value - Lines 1777, 1817: `r.Equal` assertions **Not changed:** `--upload-as localBlob` CLI flag (stays lowercase — enum value, not a type name) ## Dependencies Depends on #2323 being merged and published (descriptor/v2 with `LocalBlob` as primary type). ## Related PRs - #2323 — PR 1: descriptor/v2 + descriptor/runtime (constants, scheme, tests) - #2324 — PR 2: constructor (YAML fixtures) - **This PR** — PR 3: cli (test assertions) - #2326 — PR 4: kubernetes/controller (test JSON) Resolves part of ocm-project/ocm-project#962 --------- Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com> Co-authored-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
… [1/4] (#2323) ## Summary Renames `localBlob` → `LocalBlob` (UpperCamelCase) as the primary type name, preserving `localBlob` as a legacy alias for backward compatibility. This is the first of 4 PRs completing the `localBlob` portion of ocm-project/ocm-project#962. Follows the exact pattern established in #2057 for File, Dir, UTF8, and Helm input types. ## Changes **Production code:** - `descriptor/v2/local_access.go` — `LocalBlobAccessType = "LocalBlob"`, new `LegacyLocalBlobAccessType = "localBlob"`, added `+ocm:jsonschema-gen:enum` markers - `descriptor/v2/scheme.go` — 4 aliases: `LocalBlob/v1` (primary), `LocalBlob`, `localBlob/v1` (legacy), `localBlob` - `descriptor/runtime/local_access.go` — same constant + marker changes **Tests:** - `descriptor/v2/local_access_test.go` — marshal asserts `"LocalBlob/v1"`, added `_UpperCamelCase` unmarshal test, renamed existing unmarshal to `_LegacyType` - `descriptor/v2/scheme_test.go` — **NEW**: 4 scheme resolution tests (all alias variants: versioned/unversioned × primary/legacy) - `descriptor/v2/helpers_test.go` — added `TestIsLocalBlob_RawLegacy`, updated Raw data JSON **Generated:** - Both `schemas/LocalBlob.schema.json` now have enum constraints: `LocalBlob/v1`, `localBlob/v1`, `LocalBlob` (deprecated), `localBlob` (deprecated) ## Follow-up PRs (after this merges + publishes) These are independent of each other and can merge in parallel: - #2324 — PR 2 (constructor): update YAML `type: localBlob` → `type: LocalBlob` in test fixtures - #2325 — PR 3 (cli): update 5× `"localBlob/v1"` → `"LocalBlob/v1"` string assertions - #2326 — PR 4 (kubernetes/controller): update 13× JSON type strings + use constants in CEL test Resolves the localBlob portion of ocm-project/ocm-project#962 --------- Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com> Co-authored-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
## Summary - Rename all `localBlob/v1` references to `LocalBlob/v1` across website documentation (6 files) - Fix unversioned `localBlob` → `LocalBlob/v1` in signing concept doc - Add "Legacy alias: `localBlob`" note in the reference doc (consistent with `OCIImageLayer/v1` and `Helm/v1` style) - Remove unnecessary `Helm/v1` deprecation callout (no other type has one) ## What's unchanged (by design) - `--upload-as localBlob` CLI flag — this is an enum value, not a type name - `content_versioned/` legacy docs — intentionally frozen (same approach as #2244) ## Dependencies - **Depends on:** #2244 — this PR is rebased on top of `website-upper-camel-case-types` and should be merged after #2244 ## Related - Part of open-component-model/ocm-project#962 - Related code PRs: #2323 (descriptor), #2324 (constructor), #2325 (cli), #2326 (controller) --------- Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com> Co-authored-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
## Summary - Rename all `localBlob/v1` references to `LocalBlob/v1` across website documentation (6 files) - Fix unversioned `localBlob` → `LocalBlob/v1` in signing concept doc - Add "Legacy alias: `localBlob`" note in the reference doc (consistent with `OCIImageLayer/v1` and `Helm/v1` style) - Remove unnecessary `Helm/v1` deprecation callout (no other type has one) ## What's unchanged (by design) - `--upload-as localBlob` CLI flag — this is an enum value, not a type name - `content_versioned/` legacy docs — intentionally frozen (same approach as #2244) ## Dependencies - **Depends on:** #2244 — this PR is rebased on top of `website-upper-camel-case-types` and should be merged after #2244 ## Related - Part of open-component-model/ocm-project#962 - Related code PRs: #2323 (descriptor), #2324 (constructor), #2325 (cli), #2326 (controller) --------- Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com> Co-authored-by: Gerald Morrison (SAP) <gerald.morrison@sap.com> b742838
Summary
Updates YAML test fixtures in
bindings/go/constructor/to useLocalBlob(UpperCamelCase) instead oflocalBlob.Part of the localBlob → LocalBlob UpperCamelCase migration for ocm-project/ocm-project#962.
Changes
construct_resource_test.go: YAML fixturetype: localBlob→type: LocalBlobconstruct_source_test.go: YAML fixturetype: localBlob→type: LocalBlobDependencies
Depends on #2323 being merged and published (descriptor/v2 with
LocalBlobas primary type).Related PRs
Resolves part of ocm-project/ocm-project#962