Fix issue where delivery failures return 'true'

ManageIQ / manageiq-automation_engine

Automation engine for ManageIQ

Apache License 2.0

11 stars 74 forks source link

Fix issue where delivery failures return 'true' #475

Closed Fryguy closed 3 years ago

Fryguy commented 3 years ago

This method is expected to return nil on failures, otherwise the workspace itself, however when there is an error, _log.error will return true, which becomes the return value of this method.

coveralls commented 3 years ago

Pull Request Test Coverage Report for Build 5186

1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.002%) to 85.741%

Totals
Change from base Build 5152:	0.002%
Covered Lines:	5051
Relevant Lines:	5891

💛 - Coveralls

Fryguy commented 3 years ago

@chessbyte No, because that will fall through to line 124 (return_result), which does the right thing.

chessbyte commented 3 years ago

@Fryguy thanks - as a general observation, this method is too long. In a future PR, would be nice to refactor it so we can see the forest for the trees.

Fryguy commented 3 years ago

Backported to lasker in commit 5eeb6aaf0106910ae5eeb5c80d95d3f68ed6e06a.

commit 5eeb6aaf0106910ae5eeb5c80d95d3f68ed6e06a
Author: Oleg Barenboim <chessbyte@gmail.com>
Date:   Wed Mar 31 08:58:15 2021 -0400

    Merge pull request #475 from Fryguy/fix_deliver_return

    Fix issue where delivery failures return 'true'

    (cherry picked from commit 5be34d7779dbd7e968326076c39aacca197fc59f)