You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The first - default-branch - solution makes it for example not very safe to compare a parameter that is a hash. As this hash is just converted to a string, tests might fail on some occasions and work on other occasions. As you can not be sure, that a hash is always ordered the same way, the string of the hash from the catalogue might look differently than the string of the hash from your specs. -> While the to_s has probably been taken to avoid the issue that puppet doesn't know about Integers and internally stores them as strings, it introduces problems on more complex data types, such as hashes.
-> We should compare complex datatypes in more intelligent, way.
The second mentioned branch (comparing arrays) is similar to this issue, probably it tried to be intelligent, but fails to do so. It introduces the possibility that something matches although it should not match:
Assume that within the catalog a parameter is ['ab','b' ] but according to our spec it should be ['a','bb'], this will not fail and pass! (at least on ruby 1.8.7)
As we are simply joining the array, both values and up to be 'abb' and hence match, although they shouldn't.
-> We should compare the array differently
In general using to_s is probably a bad idea and we should compare more complex datatypes with casting it to a different type, while for numbers, strings, and so on to_s is probably still safe.
The text was updated successfully, but these errors were encountered:
Matching things in https://github.com/rodjek/rspec-puppet/blob/master/lib/rspec-puppet/matchers/create_generic.rb#L38 is likely to fail from time to time on certain circumstances.
Parameters are more or less compared as strings, due to following code parts:
https://github.com/rodjek/rspec-puppet/blob/master/lib/rspec-puppet/matchers/create_generic.rb#L59-L63
and
https://github.com/rodjek/rspec-puppet/blob/master/lib/rspec-puppet/matchers/create_generic.rb#L53-L57
The first - default-branch - solution makes it for example not very safe to compare a parameter that is a hash. As this hash is just converted to a string, tests might fail on some occasions and work on other occasions. As you can not be sure, that a hash is always ordered the same way, the string of the hash from the catalogue might look differently than the string of the hash from your specs. -> While the to_s has probably been taken to avoid the issue that puppet doesn't know about Integers and internally stores them as strings, it introduces problems on more complex data types, such as hashes.
-> We should compare complex datatypes in more intelligent, way.
The second mentioned branch (comparing arrays) is similar to this issue, probably it tried to be intelligent, but fails to do so. It introduces the possibility that something matches although it should not match:
Assume that within the catalog a parameter is ['ab','b' ] but according to our spec it should be ['a','bb'], this will not fail and pass! (at least on ruby 1.8.7)
As we are simply joining the array, both values and up to be 'abb' and hence match, although they shouldn't.
-> We should compare the array differently
In general using to_s is probably a bad idea and we should compare more complex datatypes with casting it to a different type, while for numbers, strings, and so on to_s is probably still safe.
The text was updated successfully, but these errors were encountered: