Skip to content

Minor changes to increase Coverage#185

Merged
agrare merged 10 commits into
ManageIQ:masterfrom
kbrock:coverage
May 23, 2024
Merged

Minor changes to increase Coverage#185
agrare merged 10 commits into
ManageIQ:masterfrom
kbrock:coverage

Conversation

@kbrock
Copy link
Copy Markdown
Member

@kbrock kbrock commented May 22, 2024

Timebox exercise to increase code coverage via SimpleCov

Mostly superficial test changes

one actual change to choice rule so it could show an invalid or missing compare_key

@kbrock kbrock requested review from Fryguy and agrare as code owners May 22, 2024 15:14
Comment thread lib/floe/workflow/choice_rule/data.rb Outdated

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
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll pull out of this PR

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if someone forgets or mistypes the comparison operator, then our code blows up on a nil error

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kbrock
Copy link
Copy Markdown
Member Author

kbrock commented May 22, 2024

update:

  • dropped the compare_key change

@kbrock
Copy link
Copy Markdown
Member Author

kbrock commented May 22, 2024

update:

  • rebase

no other changes/no conflicts

@kbrock
Copy link
Copy Markdown
Member Author

kbrock commented May 22, 2024

kicking (failure was due to run taking too long)

@kbrock kbrock closed this May 22, 2024
@kbrock kbrock reopened this May 22, 2024
@kbrock
Copy link
Copy Markdown
Member Author

kbrock commented May 23, 2024

update:

  • test floe log setters
  • Drop ReferencePath.get and set
  • test Context#success?
  • test workflow invalid state type
  • drop State#finished?
  • test workflow invalid payload

Comment on lines -93 to -96
def finished?
context.state.key?("FinishedTime")
end

Copy link
Copy Markdown
Member

@agrare agrare May 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@kbrock
Copy link
Copy Markdown
Member Author

kbrock commented May 23, 2024

update:

  • rebase
  • added State#finished back in with tests

update:

  • rubocop: removed trailing whitespace

@miq-bot
Copy link
Copy Markdown
Member

miq-bot commented May 23, 2024

Checked commits kbrock/floe@8b559f0~...2f504f1 with ruby 2.7.8, rubocop 1.56.3, haml-lint 0.51.0, and yamllint
9 files checked, 0 offenses detected
Everything looks fine. 🏆

@agrare agrare self-assigned this May 23, 2024
@agrare agrare merged commit 1cd4d41 into ManageIQ:master May 23, 2024
@kbrock kbrock deleted the coverage branch May 26, 2024 21:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants