Skip to content

(#22129) force explicit file content to be treated as a bytestring#1831

Merged
adrienthebo merged 2 commits intopuppetlabs:masterfrom
djmitche:bug22129
Sep 10, 2013
Merged

(#22129) force explicit file content to be treated as a bytestring#1831
adrienthebo merged 2 commits intopuppetlabs:masterfrom
djmitche:bug22129

Conversation

@djmitche
Copy link
Contributor

@djmitche djmitche commented Aug 9, 2013

This replicates the implicit behavior in Ruby-1.8.7, and avoids chasing
down every location where a value is passed to should= and setting the
encoding there.

I don't have an environment with Ruby-2.0 and puppet set up to run the new spec, but from inspection, and testing the code manually, I think it will work.

This replicates the implicit behavior in Ruby-1.8.7, and avoids chasing
down every location where a value is passed to `should=` and setting the
encoding there.
@puppetcla
Copy link

CLA signed by all contributors.

@adrienthebo
Copy link
Contributor

@Sharpie is our resident expert on Unicode support inside of Puppet, could you look this over?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be written as it "should ...", :if => "".respond_to?(:encode) do. That way it is still known by rspec even when it isn't run. https://www.relishapp.com/rspec/rspec-core/docs/filtering/if-and-unless

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed - thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

Is this invoking Puppet::Parameter#value= ? If so, why would this be done here instead of in the munge hook?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it's a local variable, passed by reference to super.

@djmitche
Copy link
Contributor Author

djmitche commented Sep 3, 2013

Any other concerns here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I traced down the travis-ci failures to this line; it looks like Ruby 2.0 was changing the encoding of the asserted string on this line. I'll fix this up locally and merge.

@adrienthebo
Copy link
Contributor

I've made a couple of fixups to this locally and it's ready to merge, but the build is currently failing (https://jenkins.puppetlabs.com/view/Puppet%20FOSS/view/Master/job/Puppet%20Specs%20(master)/1318/). As soon as the build is passing then this can be merged. Thanks again for the contribution!

adrienthebo added a commit that referenced this pull request Sep 10, 2013
(#22129) force explicit file content to be treated as a bytestring
@adrienthebo adrienthebo merged commit 4e448c8 into puppetlabs:master Sep 10, 2013
@adrienthebo
Copy link
Contributor

Wait. Shoot. I didn't mean to merge that one. I was going to merge my local fixup one. Well, there goes the build. :|

@adrienthebo
Copy link
Contributor

Merged the spec fixture fix in 9a24fa8. This should be released in 3.4.0, thanks again!

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.

4 participants