-
Notifications
You must be signed in to change notification settings - Fork 34
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
Support macOS 11.0 and 11 builds; Update tests #503
Conversation
- refresh the mock data from artifactory - added helper to easily refresh data going forward - update tests with accurate versions (e.g., versions may no longer exist in the current channel) - Add helper logic to avoid having to manually update a bunch of spec tests - Use `mixlib/install` to fetch latest version data rather than hard-coding it Signed-off-by: Tom Duffield <tom@chef.io>
Signed-off-by: Tom Duffield <tom@chef.io>
Updates our comparison logic to handle the situation caused by having both 11 and 11.0 builds. If we're comparing two versions, and one has a minor/patch while the other does not, simply stop comparing and use the last known comparsion. Signed-off-by: Tom Duffield <tom@chef.io>
Signed-off-by: Tom Duffield <tom@chef.io>
998ece7
to
4ab3e8d
Compare
lib/platform_dsl.rb
Outdated
ret = favor_integer_sorting(self.mapped_minor, otherVer.mapped_minor) if ret == 0 && self.mapped_minor | ||
ret = favor_integer_sorting(self.mapped_patch, otherVer.mapped_patch) if ret == 0 && self.mapped_patch | ||
if ret == 0 && self.mapped_minor | ||
return ret if otherVer.mapped_minor.nil? |
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.
only question i've got here is if its deliberate that "11" also matches "11.2"? that does seem to work in my head due to yolo mode and partial version mappings, but its not quite called out in the tests that nil now means "match any minor number" rather than just matching zero (at least if I'm also reading the code correctly i suppose). probably worth a comment and/or a test at any rate.
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.
Sure. The intention is that it would support 11.2. I'll add some comments/tests to make that more explicit.
Signed-off-by: Tom Duffield <tom@chef.io>
Signed-off-by: Tom Duffield <tom@chef.io>
63b954f
to
06882e4
Compare
Updates our comparison logic to handle the situation caused by having both 11 and 11.0 builds. If we're comparing two versions, and one has a minor/patch while the other does not, simply stop comparing and use the last known comparison.
This PR also updates the tests to pull in the latest data. As part of this I improved the fixture handling around our expectations to reduce the number of places we'd need to manually update strings when new data was brought in.
Closes #502
Signed-off-by: Tom Duffield tom@chef.io