Skip to content
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

Make force_os/force_dist work properly with debian repos #41

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

ctdk
Copy link
Contributor

@ctdk ctdk commented Apr 14, 2016

Fixes the issue mentioned in #40 where the force_os/force_dist options don't actually work for deb repos.

One note: in raintank@27080c1#diff-2bb89c3cb0524511e4c762ffc09cf060R21, one can see that the test installs a debian wheezy repo from computology/packagecloud-test-packages and installs packagecloud-test for debian wheezy. Since this does not actually currently exist in that repo it will fail (I had to test with a goiardi repo to make sure it worked, but it didn't seem appropriate for the PR). Someone over there will need to set that up.

@yourivdlans
Copy link

Would love to see this merged :) what do we need to get it merged?

force_dist 'wheezy'
end

package 'packagecloud-test'
Copy link
Contributor

Choose a reason for hiding this comment

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

this packageline causes the tests to fail

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, I see. I missed your comment on the PR, sorry.

@ice799
Copy link
Contributor

ice799 commented Mar 8, 2017

So I think perhaps a different test we could add instead of installing a package here would be to check the apt repo config against a regex or something, since that is essentially what we want to verify -- that the repo config was properly written out given a forced OS/dist.

@ice799 ice799 merged commit 27080c1 into sous-chefs:master Mar 8, 2017
@ice799
Copy link
Contributor

ice799 commented Mar 8, 2017

OK, I modified this slightly to use a different test and merged it. Thanks @ctdk for this PR. Closing this out now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants