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

Deep freeze only strings, arrays and hashes #447

Merged
merged 1 commit into from Jan 27, 2017

Conversation

Projects
None yet
4 participants
@domcleal
Contributor

domcleal commented Jan 27, 2017

Freezing a class prevents it from being modified at all, which can
happen in unit tests with mocha or rspec-expections.

Calling a function with String as an argument will freeze the entire
class and prevent allow_any_instance_of(String)... type expectations
from adding their hooks to the class.

e.g. stdlib:
https://github.com/puppetlabs/puppetlabs-stdlib/blob/00c881d/spec/functions/is_a_spec.rb#L18
https://github.com/puppetlabs/puppetlabs-stdlib/blob/00c881d/spec/functions/pw_hash_spec.rb#L57

This whitelists strings, arrays and hashes as safe classes to freeze
when testing functions, others are left thawed to prevent side effects.
Deep freezing is used on arrays/hashes to match Puppet's behaviour of
freezing facts.


Alternative to #446 with a whitelisted set of classes instead of blacklisting Class specifically. This is closer to Puppet's implementation at https://github.com/puppetlabs/puppet/blob/master/lib/puppet/parser/scope.rb#L775, per the discussion on #442.

@coveralls

This comment has been minimized.

coveralls commented Jan 27, 2017

Coverage Status

Coverage increased (+0.04%) to 92.025% when pulling 06d593f on domcleal:no-freeze-classes-whitelist into b381c0e on rodjek:master.

Deep freeze only strings, arrays and hashes
Freezing a class prevents it from being modified at all, which can
happen in unit tests with mocha or rspec-expections.

Calling a function with `String` as an argument will freeze the entire
class and prevent `allow_any_instance_of(String)...` type expectations
from adding their hooks to the class.

e.g. stdlib:
  https://github.com/puppetlabs/puppetlabs-stdlib/blob/00c881d/spec/functions/is_a_spec.rb#L18
  https://github.com/puppetlabs/puppetlabs-stdlib/blob/00c881d/spec/functions/pw_hash_spec.rb#L57

This whitelists strings, arrays and hashes as safe classes to freeze
when testing functions, others are left thawed to prevent side effects.
Deep freezing is used on arrays/hashes to match Puppet's behaviour of
freezing facts.

@domcleal domcleal force-pushed the domcleal:no-freeze-classes-whitelist branch from 06d593f to 351371b Jan 27, 2017

@coveralls

This comment has been minimized.

coveralls commented Jan 27, 2017

Coverage Status

Coverage increased (+0.04%) to 92.021% when pulling 351371b on domcleal:no-freeze-classes-whitelist into b381c0e on rodjek:master.

@DavidS DavidS merged commit 9c82100 into rodjek:master Jan 27, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@rodjek rodjek added the bugfix label Jul 25, 2017

@rodjek rodjek added this to the 2.6.0 milestone Jul 25, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment