ruby
Document unit tests (issue #6)
- adds documentation for unit test methods and constants
- adds an explanatory note to the benchmark.rb file
- adds an IMPLEMENTATION.md file with an explanation of label width handling
- fixes a misspelling in a test method name
In general, test suites for the Ruby standard library do not document what is being tested to this level. That could be considered a good thing (less noise when updating tests) or bad thing (hard to understand what is being tested) depending on your point of view, but this change would make benchmark inconsistent with most of the rest of the standard library. If you think we should start documenting standard library tests like this, you should probably create a Misc ticket on the Ruby bug tracker, and add it to the list of topics to be discussed at the next developer meeting.
I think the change made in lib is good, and fixing misspelled test names is also a good change to make. So those changes we could easily merge separately if you submit a separate pull request for them.
Hi, Jeremy. Regarding the test documentation, it took me a while to figure out everything that was going on; there was often more information that needed to be known than could be fit in the test name, and nuances that were not readily apparent. I figured the documentation would be helpful to someone else wanting to understand the tests and even the lib code it tested.
Personally, if it were me, I would welcome a well documented file even if it were inconsistent with less- or undocumented code elsewhere in the code base, assuming the documentation was truly helpful and not mere proforma verbosity.
Although documentation can get stale quickly, this code has been very stable over the years.
That said, I will follow your direction and make a separate PR for the test name fix and the lib file. Regarding the documentation in the new IMPLEMENTATION.md file, I will assume you don't want to keep it or move it somewhere else unless you tell me otherwise.
Thanks for looking into this.
On Mon, Mar 1, 2021 at 5:17 PM Jeremy Evans [email protected] wrote:
In general, test suites for the Ruby standard library do not document what is being tested to this level. That could be considered a good thing (less noise when updating tests) or bad thing (hard to understand what is being tested) depending on your point of view, but this change would make benchmark inconsistent with most of the rest of the standard library. If you think we should start documenting standard library tests like this, you should probably create a Misc ticket on the Ruby bug tracker, and add it to the list of topics to be discussed at the next developer meeting.
I think the change made in lib is good, and fixing misspelled test names is also a good change to make. So those changes we could easily merge separately if you submit a separate pull request for them.
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ruby/benchmark/pull/7#issuecomment-788340389, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAG56SBJBUV5C276XZSINTTBQG7BANCNFSM4XP7QA5A .