Minor changes to increase Coverage #185
Conversation
| @@ -81,11 +81,11 @@ def is_timestamp?(value) # rubocop:disable Naming/PredicateName | |||
| end | |||
|
|
|||
| def compare_key | |||
| @compare_key ||= payload.keys.detect { |key| key.match?(/^(IsNull|IsPresent|IsNumeric|IsString|IsBoolean|IsTimestamp|String|Numeric|Boolean|Timestamp)/) } | |||
| @compare_key ||= (payload.keys - %w(Variable Next)).first | |||
There was a problem hiding this comment.
I'm a little worried about moving from a list of known keys to everything except these two. For one thing I think there is a Default key that is optional that we don't implement yet, and if someone mistype'd something it could get picked up here.
Would be nice to move this list of known keys to a constant and verify the payload on initialize instead of on evaluation like we have now.
There was a problem hiding this comment.
if someone forgets or mistypes the comparison operator, then our code blows up on a nil error
|
update:
|
|
update:
no other changes/no conflicts |
|
kicking (failure was due to run taking too long) |
|
update:
|
lib/floe/workflow/state.rb
Outdated
| def finished? | ||
| context.state.key?("FinishedTime") | ||
| end | ||
|
|
There was a problem hiding this comment.
Hm even if we don't end up calling this internally I think it is a nice interface method for external callers (symmetry with https://github.com/ManageIQ/floe/pull/185/files#diff-24e71eb9cd8cfb4211a041b7a08cfc1d9d33203a6c2ce88dbc153234e82dfe1cR85-R87)
last usage: f019789
This is only used by bin/floe
|
update:
update:
|
|
Checked commits kbrock/floe@8b559f0~...2f504f1 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint |
Timebox exercise to increase code coverage via
SimpleCovMostly superficial test changes
one actual change to choice rule so it could show an invalid or missing compare_key