Improvements to specs for Random::Formatter#1228
Improvements to specs for Random::Formatter#1228herwinw wants to merge 3 commits intoruby:masterfrom
Conversation
The module does still exist, but only if you require `securerandom` instead of `random/formatter`.
* Depend on the instance method `bytes` * Ensure the output is the same if the input bytes are the same * Ensure the output is not the same if the input bytes are not the same Right now, this is only used in the specs for `alphanumeric`, since this is the only method of Random::Formatter with specs, but this should be used in every other spec we add to this folder, since the specs have no dependency on the exact format that is being used.
herwinw
changed the title
| bytes.extend(Random::Formatter) | ||
| bytes.should_receive(:bytes).with(4).exactly(4).and_return("\x00".b * 4) | ||
| bytes.send(@method) | ||
| end |
There was a problem hiding this comment.
It seems to me like an implementation detail - in what way the #bytes method is called.
Moreover these 4x4 calls approach is specific to the #alphanumeric method e.g. #base64 calls by default #bytes only once:
obj = Object.new
def obj.bytes(n)
puts "#bytes(#{n})"
"\x00".b*n
end
obj.singleton_class.include(Random::Formatter)
obj.base64
#bytes(16)
# => "AAAAAAAAAAAAAAAAAAAAAA=="And #alphanumeric behaviour depends on n:
obj.alphanumeric(8)
#bytes(4)
#bytes(4)
# => "AAAAAAAA"So I would check only that the interface between the Random::Formatter module and a class that includes it is just a single method #bites, and nothing more is required.
| end | ||
| lhs.send(@method).should != rhs.send(@method) | ||
| end | ||
| end |
There was a problem hiding this comment.
subjective: TBH I don't see value in the last two test cases.
I would keep only one test case (that checks that a #bytes method is called and move it to the #alphanumeric specs file (and don't have the shared example at all))
|
@herwinw Could you fix the conflict and address @andrykonchin's review and merge this yourself? |
3 participants
This addresses most of the open issues left in #1222. Most relevant is the shared spec to get a definition of what all methods on
Random::Formattershould behave like. Once this is finished, I will start with moving the relevant specs fromSecureRandomto this new folder.