fix(salesforce-api): #3938 RestDeploy raises on deploy failure instead of silently succeeding#3983
Draft
jstvz wants to merge 2 commits into
Draft
fix(salesforce-api): #3938 RestDeploy raises on deploy failure instead of silently succeeding#3983jstvz wants to merge 2 commits into
jstvz wants to merge 2 commits into
Conversation
…n failure
Adds TestRestDeployRaisesOnFailure with three failing tests that pin down the
silent-success bug in cumulusci/salesforce_api/rest_deploy.py:
- test_rest_deploy_raises_on_failure: _monitor_deploy_status must raise
MetadataApiError when deployResult.status == "Failed".
- test_rest_deploy_raises_component_failure_subclass: failures with
componentFailures should raise MetadataComponentFailure (subclass of
MetadataApiError), matching SOAP ApiDeploy._process_response behaviour.
- test_rest_deploy_raises_on_initial_post_failure: __call__ must raise
MetadataApiError when the initial POST returns non-201.
All three tests fail on unmodified source (DID NOT RAISE). This is the RED
half of the TDD fix for #3938.
Refs #3938
…s raise on failure
Before this change, the REST metadata-deploy path silently returned success
when Salesforce reported a deployment failure:
- RestDeploy.__call__ logged "Deployment request failed with status code
{n}" for any non-201 response and then returned normally.
- RestDeploy._monitor_deploy_status logged each componentFailure entry and
then returned None when deployResult.status == "Failed".
As a result, `cci task run deploy --rest_deploy True` exited 0 even on a hard
deploy failure, masking errors from CI/CD.
This commit aligns RestDeploy with the SOAP path
(cumulusci/salesforce_api/metadata.py::ApiDeploy._process_response):
- Non-201 initial POST raises MetadataApiError with the status code and
response body included in the message.
- "Failed" deployResult raises MetadataComponentFailure when the response
has componentFailures (subclass of MetadataApiError), otherwise raises
MetadataApiError. Per-component errors are still logged before the raise
so log output stays informative.
The three regression tests added in the prior RED commit now pass.
Pre-existing tests updated to reflect the corrected behaviour (they
previously encoded the buggy silent-success contract):
- TestRestDeploy.test_deployment_failure: now expects MetadataApiError.
- TestRestDeploy.test_deployStatus_failure: now expects
MetadataComponentFailure.
Full cumulusci/salesforce_api/ suite (295 tests) plus
cumulusci/tasks/salesforce/tests/test_Deploy.py (26 tests) all pass.
Fixes #3938
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
1 participant
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.
Summary
Fixes #3938.
cumulusci/salesforce_api/rest_deploy.pywas returning successfully on failed REST-API deployments because_monitor_deploy_statusand__call__did not raise on terminal failure states. Errors surfaced as warnings in the log and downstream flow steps continued as if the deploy had succeeded.The fix raises
MetadataApiError(orMetadataComponentFailurewhen component-level details are available) when a REST deployment ends inFailed/Canceled, matching the long-standing SOAP-API path.Test plan
cumulusci/salesforce_api/tests/test_rest_deploy.py::TestRestDeployRaisesOnFailure3 tests pass (new).TestRestDeploy.test_deployment_failureandTestRestDeploy.test_deployStatus_failureupdated to assert the new raise contract.cumulusci/tests/triage/test_issue_3938.pyremoved in the GREEN commit; test now passes.uv run pytest cumulusci/salesforce_api/tests/test_rest_deploy.py -qclean.Provenance
Reproduced and characterized in the v5 triage evidence pack (PR #3979). See
docs/triage/v5/repro-results.md(### #3938) for the full narrative.Companion to #3939 (same author, same day, same task family; SOAP-side exception preservation).