Improvements to specs for Random::Formatter #1228
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.