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

Added mode, owner_user, and owner_group methods to file #544

Merged
merged 1 commit into from
Jan 9, 2016

Conversation

twolfson
Copy link
Contributor

When writing tests, I was getting frustrated at the poor feedback I was getting for mode, user, and group. For example, in the error below, there is no information about what the current mode is:

expected `File "/etc/ssl/private/twolfson.com.key".mode?("300")` to return true, got false

I saw that we have an mtime method and while I don't know its motivations (kind of hard with no GitHub issues or related PR in the blame =/). However, I gave a shot at adding in mode, owner_user, andowner_group`. In this PR:

  • Added mode, owner_user, and owner_group methods to file
  • Added tests

@twolfson twolfson force-pushed the dev/add.more.file.methods.sqwished branch from 085682f to 4388d26 Compare December 10, 2015 08:44
@@ -338,10 +338,25 @@
end

describe file('/etc/passwd') do
let(:stdout) { "644\r\n" }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to avoid using let(:stdout) but couldn't seem to get it working otherwise. I'm still not sure how the be_mode and be_owned_by matchers are working =/

@mizzy
Copy link
Owner

mizzy commented Dec 10, 2015

Thanks!

But it would be better that owner_user and owner_group are owner and group
because Chef and Puppet use these words.

https://docs.puppetlabs.com/references/latest/type.html#file
https://docs.chef.io/resource_file.html

@twolfson
Copy link
Contributor Author

Ah, k. I chose those to keep things consistent in the repo. We could also use an attr_alias but I will update the names to owner/group for now =)

@twolfson twolfson force-pushed the dev/add.more.file.methods.sqwished branch from 4388d26 to 2988094 Compare December 10, 2015 18:11
@twolfson
Copy link
Contributor Author

I have applied the updates to this PR

@mizzy
Copy link
Owner

mizzy commented Dec 13, 2015

Thanks!

One more thing, it would be useful that mode is octal, not string.

@twolfson
Copy link
Contributor Author

Ah, sure thing. I will update that.

@twolfson
Copy link
Contributor Author

I started moving to an octal but the error messages from failing test suites have become less useful:

     Failure/Error: its(:mode) { should eq 00644 }

       expected: 420
            got: "644"

The 420 in that statement is 00644 being parsed by Ruby. I suggest we rethink moving from octal to string. If someone wants to use an actual octal, then 00644.to_s should suffice.

@mizzy
Copy link
Owner

mizzy commented Dec 14, 2015

Ah, let me rethink about it. Thanks for reporting!

@twolfson
Copy link
Contributor Author

Any new thoughts on this?

@twolfson twolfson force-pushed the dev/add.more.file.methods.sqwished branch from 2988094 to 5ae13e5 Compare December 24, 2015 18:12
@mizzy
Copy link
Owner

mizzy commented Jan 8, 2016

Sorry for letting you wait. I didn't think of any good ideas. So I'd like to merge current code.

Could you fix this line to pass tests, please ?
https://github.com/twolfson/serverspec/blob/dev/add.more.file.methods.sqwished/spec/type/base/file_spec.rb#L352

@twolfson
Copy link
Contributor Author

twolfson commented Jan 8, 2016

Oh, heh. I didn't realize I broke the build. Yep, I will push a patch shortly.

@twolfson twolfson force-pushed the dev/add.more.file.methods.sqwished branch from 5ae13e5 to 2d8147d Compare January 8, 2016 17:59
@twolfson
Copy link
Contributor Author

twolfson commented Jan 8, 2016

Alright, I have updated the code so it should be green in Travis CI now

@mizzy
Copy link
Owner

mizzy commented Jan 9, 2016

Thanks!

mizzy added a commit that referenced this pull request Jan 9, 2016
Added mode, owner_user, and owner_group methods to file
@mizzy mizzy merged commit c609427 into mizzy:master Jan 9, 2016
@mizzy
Copy link
Owner

mizzy commented Jan 9, 2016

Released as v2.29.0.

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.

2 participants