-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Fix tests for #13733 #13740
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
Fix tests for #13733 #13740
Conversation
This was done because the --inventory-file option for Ansible is anticipated to be removed as of the ansible-core package version '2.23'.
chrisroberts
left a comment
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.
Looks great!
…on the ansible-core version
ae821ae to
dc373e4
Compare
chrisroberts
left a comment
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.
These updates look great! Left a couple simplification suggestions and one note on a conditional that should be adjusted (or removed).
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.
Pull Request Overview
This PR fixes test failures introduced in #13733 by updating tests and implementation to handle ansible-core version detection alongside ansible version detection, and switches to the --inventory flag for ansible-core >= 2.19.
Key changes:
- Updates box_add test to use asterisk masking pattern for credentials in HTTP URLs
- Adds ansible-core version checking to the provisioner alongside existing ansible version checks
- Implements version-aware inventory argument selection (--inventory vs --inventory-file)
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/vagrant/action/builtin/box_add_test.rb | Updates credential masking pattern in HTTP URL test assertion |
| test/unit/plugins/provisioners/ansible/provisioner_test.rb | Adds test cases for ansible-core version checking and inventory argument selection based on version |
| plugins/provisioners/ansible/provisioner/host.rb | Makes gather_ansible_version accept package parameter for querying different ansible packages |
| plugins/provisioners/ansible/provisioner/guest.rb | Makes gather_ansible_version accept package parameter for querying different ansible packages |
| plugins/provisioners/ansible/provisioner/base.rb | Implements ansible-core version tracking and version-based inventory argument selection |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chrisroberts
left a comment
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.
Looks good!
Fixing test failures in #13733