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
This is going to take a bit of time, so I would like to have an approval for the idea before I actually spend a few hours on this.
Ruby has two libraries to calculate digests: digest and openssl (or the classes Digest en OpenSSL::Digest). The tests for openssl-digest currently use a lot of the constants of the digest, but they could use the specs as well. The current pull request is a little proof of concept for a single spec.
That brings us to the next point: the documentation of the OpenSSL::Digest class is weird. When I look at https://www.rubydoc.info/stdlib/openssl/OpenSSL/Digest, the only documented method is digest. https://docs.ruby-lang.org/en/3.2/OpenSSL/Digest.html describes a few more methods. But it turns out that the OpenSSL::Digest object had methods size and length too, similar to the Digest object. Or the OpenSSL::Digest object has a file method too, similar to Digest, but this is not documented. Should these undocumented methods be added to the specs too?
So in terms of testing coverage I don't think it's so valuable to do that work, i.e., it should be enough to test Digest::Class and Digest::Instance methods once for a given digest, and just ensure all digests include those two modules.
In fact there are already some specs for Digest::Instance like library/digest/instance/new_spec.rb. They can just use Digest::MD5.new to have a concrete Digest to test. There doesn't seem to be specs for Digest::Class itself but several files under library/digest/ are actually testing Digest::Instance and Digest::Class methods.
Ideally the specs would always be defined in a file matching the module owning them and not subclasses/classes including the module.
This file contains hidden or 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
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 is going to take a bit of time, so I would like to have an approval for the idea before I actually spend a few hours on this.
Ruby has two libraries to calculate digests:
digestandopenssl(or the classesDigestenOpenSSL::Digest). The tests for openssl-digest currently use a lot of the constants of the digest, but they could use the specs as well. The current pull request is a little proof of concept for a single spec.That brings us to the next point: the documentation of the OpenSSL::Digest class is weird. When I look at https://www.rubydoc.info/stdlib/openssl/OpenSSL/Digest, the only documented method is
digest. https://docs.ruby-lang.org/en/3.2/OpenSSL/Digest.html describes a few more methods. But it turns out that the OpenSSL::Digest object had methodssizeandlengthtoo, similar to the Digest object. Or the OpenSSL::Digest object has afilemethod too, similar to Digest, but this is not documented. Should these undocumented methods be added to the specs too?