Fix phantom schemas, wrong error codes, and copy-paste bugs in feature files (#53) - #62
Merged
Merged
Conversation
…e files (camaraproject#53) Point 1 - Phantom schema references: createAppInstance.feature, createAppDeployment.feature, and updateAppDeployment.feature referenced nonexistent components (AppInstanceManifest, AppDeploymentManifest). The actual request bodies for these operations are anonymous inline object schemas in the spec, not named components. Replaced with "the request body schema for this operation" generic phrasing. Point 2 - Query parameters mislabeled as path parameters: appId, appInstanceId, region (getAppInstance.feature), appId, appDeploymentId (getAppDeployments.feature), and region, status (getEdgeCloudZones.feature) are all `in: query` in the spec but were described as "path parameter". Also fixed getAppInstance.feature's region scenario, which said "is set to a valid application ID" (copy-pasted from the appId scenario) instead of "a valid region". Point 3 - Non-existent operation names: getAppInstance.feature and getAppDeployments.feature referenced "removeAppInstance" and "removeAppDeployment", neither of which exist; corrected to deleteAppInstance/deleteAppDeployment. Point 4 - getAppInstance.feature's 410 scenario didn't apply to this operation at all: GET /appinstances doesn't document 410 (only /deployments and the two notification callbacks do), it referenced appDeploymentId instead of appInstanceId, called the nonexistent removeAppInstance operation, and had a leftover "#/deployments GET 410" comment. Removed the scenario entirely - clearly copied from the deployments feature file and never adapted. Point 5 - getAppDeployments.feature filtered by appInstanceId, which is not a valid query parameter for GET /deployments (only appId and appDeploymentId are); replaced with a filter-by-appDeploymentId scenario using the operation's actual query parameters, and fixed several createAppInstance-labeled Given/response-property steps that should have said createAppDeployment. Point 6 - Deprecated/wrong error code CONFLICT (per the Design Guide, CONFLICT is deprecated) asserted in submitApp.feature, createAppInstance.feature, and createAppDeployment.feature's 409 scenarios: corrected to ALREADY_EXISTS, matching what the spec's own 409 example documents for each of these operations. deleteApp.feature corrected to ABORTED, matching its spec example. Point 7 - Miscellaneous typos: 7a. submitApp.feature: "$.appPorvider" -> "$.appProvider", "$.componentSpect" -> "$.componentSpec"; non-standard "When invoking with the POST method..." step replaced with the standard 'When the request "submitApp" is sent' phrasing used everywhere else. 7b. createAppInstance.feature / createAppDeployment.feature: PascalCase "$.KubernetesClusterRef" corrected to the spec's actual lowerCamelCase property names: kubernetesClusterRef (singular, instances) / kubernetesClusterRefs (plural, deployments). 7c. updateAppDeployment.feature / deleteAppDeployment.feature: singular "/deployment/{appDeploymentId}" resource path corrected to plural "/deployments/{appDeploymentId}"; "$.appDeploymentIdd" typo (extra "d") fixed in both files. 7d. updateAppDeployment.feature: success scenario checked the response against AppInstanceInfo; corrected to AppDeploymentInfo, matching the spec's actual 200 response schema for this operation. 7e. getAppInstance.feature (and, for consistency, also getAppDeployments.feature): list-all/filtered success scenarios checked the response against a bare AppInstanceInfo/ AppDeploymentInfo object; GET /appinstances and GET /deployments actually return arrays of these schemas, so introduced "the response body is an array complying with the OAS schema at ..." phrasing for these array responses. Out of scope (left untouched, already covered by issue camaraproject#52's fix in a separate branch): the wrong singular "getEdgeCloudZone" operationId and the "When When the request..." keyword typo in getEdgeCloudZones.feature; the wrong "deleteApp"/"createApp" operationIds referenced in updateAppDeployment.feature, deleteAppDeployment.feature, and (partially, before I touched it for unrelated fixes) createAppInstance.feature. Fixes camaraproject#53
DLondonoD
requested review from
FabrizioMoggio,
JoseMConde,
Kevsy,
gainsley,
maheshc01 and
seralogar
as code owners
July 24, 2026 15:45
Adopt approved-but-unmerged fixes from fix/issue-52 (PR camaraproject#61 review feedback) ahead of PR camaraproject#62 to avoid future conflicts. Resolved 3 overlapping conflicts, keeping the correct value from each side based on the OpenAPI spec: - createAppInstance.feature: kept issue-53's lowerCamelCase '$.kubernetesClusterRef' (matches the spec property name) over issue-52's untouched PascalCase '$.KubernetesClusterRef'. - deleteAppDeployment.feature: combined issue-53's fix of the '$.appDeploymentIdd' typo with issue-52's fix of the wrong 'deleteApp' operationId -> 'deleteAppDeployment'. - getEdgeCloudZones.feature: combined issue-53's query-parameter fix (region/status) with issue-52's fix of the duplicated 'When When' keyword and the singular 'getEdgeCloudZone' operationId/tags, and kept the removal of the phantom 404 scenario (not documented for this operation).
Adopts main's PR camaraproject#58 restructuring of getAppInstance/getAppDeployments (list vs by-id split) ahead of PR camaraproject#62 to avoid conflicts. Resolved 2 conflicts: - getAppInstance.feature: took main's version entirely. PR camaraproject#58 split this into a by-id-only operation (new getAppInstances.feature now owns the list/filter scenarios), making fix/issue-53's list-oriented fixes to this file obsolete. - getAppDeployments.feature: kept fix/issue-53's naming/wording fixes (createAppDeployment references, 'query parameter' phrasing, array schema check) but dropped the appDeploymentId-filter scenario and its 404, since GET /deployments no longer accepts that query param per main (moved to the new by-id getAppDeployment.feature); renumbered the remaining 404 scenario back to 404.1.
getAppDeployments, deleteAppDeployment and updateAppDeployment no longer document 410 in the spec since the fix in camaraproject#48/camaraproject#56 (410 is now scoped to the notification/callback flow only). These 3 .feature files still had leftover '410 GONE' scenarios asserting a response code the operations never return.
Issue camaraproject#53 replaced the deprecated CONFLICT error code with ALREADY_EXISTS (and ABORTED for deleteApp) in the response bodies, but the scenario tags in 4 .feature files still referenced the old 'conflict' wording: - createAppInstance.feature: @eam_createAppInstance_409.conflict -> @eam_createAppInstance_409.1_already_exists (also fixed missing '.1_' numbering, inconsistent with the rest of the suite) - createAppDeployment.feature: @eam_createAppDeployment_409.conflict -> @eam_createAppDeployment_409.1_already_exists (same numbering fix) - submitApp.feature: @eam_submitApp_409.1_conflict -> @eam_submitApp_409.1_already_exists - deleteApp.feature: @eam_deleteApp_409.1_conflict -> @eam_deleteApp_409.1_aborted (matches its spec example, ABORTED not ALREADY_EXISTS)
seralogar
reviewed
Jul 31, 2026
Co-authored-by: Sergi <sergialonsogarcia@gmail.com>
Commit 725880e applied seralogar's suggestion to change deleteApp's 409 $.code from ABORTED to INCOMPATIBLE_STATE (per camaraproject#56, ABORTED is now only correct for updateAppDeployment's 409), but the scenario tag was not part of that suggestion diff. Renaming @eam_deleteApp_409.1_aborted -> @eam_deleteApp_409.1_incompatible_state to match, as also requested in the same review comment.
seralogar
approved these changes
Jul 31, 2026
DLondonoD
added a commit
to DLondonoD/EdgeApplicationManagement
that referenced
this pull request
Jul 31, 2026
Syncs with main after PR camaraproject#62 (fix/issue-53) was merged.
This was referenced Jul 31, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What type of PR is this?
tests
What this PR does / why we need it:
Fixes multiple test bugs in
Test_definitions/*.featurefiles where Gherkinsteps reference schemas, parameters, operations, or error codes that don't
match the OpenAPI spec:
createAppInstance.feature,createAppDeployment.feature, andupdateAppDeployment.featurereferenced nonexistent components (
AppInstanceManifest,AppDeploymentManifest); the actual request bodies are anonymous inlineobject schemas. Replaced with generic "request body schema for this
operation" phrasing.
appId,appInstanceId,region(getAppInstance.feature),appId,appDeploymentId(getAppDeployments.feature), andregion,status(
getEdgeCloudZones.feature) are allin: queryin the spec but weredescribed as "path parameter". Also fixed a copy-pasted
regionscenario that said "a valid application ID" instead of "a valid region".
removeAppInstance/removeAppDeployment(don't exist) corrected todeleteAppInstance/deleteAppDeployment.410scenario removed fromgetAppInstance.feature—GET /appinstancesdoesn't document410at all; the scenario wascopied from the deployments feature file and never adapted (wrong
entity id, nonexistent operation, leftover comment from the other file).
getAppDeployments.featurefiltered byappInstanceId, which isn't a valid query parameter forGET /deployments(onlyappId/appDeploymentIdare); replaced witha proper
appDeploymentIdfilter scenario, and fixed severalcreateAppInstance-labeled steps that should have saidcreateAppDeployment.CONFLICT(deprecated per theDesign Guide) asserted in
submitApp,createAppInstance, andcreateAppDeployment's409scenarios; corrected toALREADY_EXISTS,matching the spec's own examples.
deleteApp.featurecorrected toABORTED, also matching its spec example.appPorvider→appProvider,componentSpect→componentSpec, non-standard step phrasing insubmitApp.feature; PascalCaseKubernetesClusterRefcorrected to thespec's actual
kubernetesClusterRef/kubernetesClusterRefs; singular/deployment/{appDeploymentId}path corrected to plural/deployments/{appDeploymentId};appDeploymentIddtypo; wrongAppInstanceInfoschema inupdateAppDeployment.featurecorrected toAppDeploymentInfo; list-response scenarios ingetAppInstance.featureand
getAppDeployments.featurecorrected to assert against an array ofthe schema instead of a bare object.
Out of scope / left untouched: the wrong singular
getEdgeCloudZoneoperationId, the
"When When the request..."typo, and the wrongdeleteApp/createAppoperationIds referenced inupdateAppDeployment.feature,deleteAppDeployment.feature, andcreateAppInstance.feature— these are already covered by issue #52's fixin a separate branch/PR.
Which issue(s) this PR fixes:
Fixes #53
Special notes for reviewers:
This PR has two merge-order dependencies on other open PRs. Both were
verified with an actual local merge simulation (not just static analysis),
and real Git conflicts were confirmed in both cases:
1. PR #56 (fixes #48) removes
410fromgetAppDeployments,deleteAppDeployment, andupdateAppDeployment(replacing it with404on the GET, and simply dropping it where
404already existed on theother two). This PR intentionally left the three corresponding
@eam_*_410.1_gonescenarios untouched, since as of this branch's base thespec still documents
410for those operations. Once #56 merges, thosethree scenarios will test an error code that no longer exists there and
should be removed or converted to
404as follow-up.2. PR #58 (fixes #49) repurposes
getAppInstance.featurefrom thelist operation (
GET /appinstances) to a new by-id operation(
GET /app-instances/{appInstanceId}), moving the list scenarios into anew
getAppInstances.featurefile, and removesgetAppDeployments'appDeploymentIdquery filter entirely (since it becomes redundant witha new
getAppDeploymentby-id endpoint). This directly conflicts withthis PR's changes:
-
getAppInstance.feature: this PR fixes it as the list operation(query-param labels, array-schema assertion, removed invalid
410scenario); after #58, that content semantically belongs in
getAppInstances.featureinstead, andgetAppInstance.featurewillmean something entirely different (by-id).
-
getAppDeployments.feature: this PR adds a "filtered byappDeploymentId" scenario (replacing the invalidappInstanceIdone); #58 removes that same filter as redundant with its new
by-id endpoint — the two changes are directly opposed.
Recommendation: merge #58 (and ideally #56) first, then rebase/redo
this PR's
getAppInstance.featureandgetAppDeployments.featurefixesagainst the post-#58 file layout (i.e., apply the equivalent fixes to
getAppInstances.featureinstead ofgetAppInstance.feature, and drop thenow-redundant
appDeploymentIdfilter addition). The other 7 files touchedby this PR (
submitApp,deleteApp,createAppInstance,createAppDeployment,deleteAppDeployment,updateAppDeployment,getEdgeCloudZones) are unaffected by either #56 or #58 and can landindependently.
Changelog input
release-note
Additional documentation
This section can be blank.
docs