-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dev 4821/add a bundle to check if a package is installed implementation test case #51
Conversation
defaults: | ||
"action" string => "immediate", if_match_regex => "add"; | ||
"action" string => "warn_only", if_match_regex => "verify|"; | ||
"policy" string => "addupdate", if_match_regex => "true"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
policy is not a parameter of the bundle, so this will never evaluate
Can you re-review please? |
vars: | ||
"suffix" slist => { "ok", "not_ok", "repaired", "kept", "error", "reached" }; | ||
classes: | ||
"ok" expression => "package_check_installed_${init.c_package_name}_ok.package_check_installed_${init.c_package_name}_reached.!package_check_installed_${init.c_package_name}_not_ok.!package_check_installed_${init.c_package_name}_repaired"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'd be much happier if the test also tested that the package was installed (dpkg -l, or equivalent, or checking the existence of a file on a file system), to test more than that the generic methods did return the correct class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I'll add that, and also add a test to ensure that the package doesn't get changed by this method, which it shouldn't since it is a check only.
Appart from the test, it looks great to me ! |
…ntation + test case)
… sure we're testing on an installed package
I've added the test you suggested, and rebased this PR on master. The change is in a separate commit, to ease your reviewing. I tested this on Debian 7 and the test passes. Please merge if this is OK! |
Awesome ! thank you |
…if_a_package_is_installed_implementation_test_case Dev 4821/add a bundle to check if a package is installed implementation test case
No description provided.