digitalmars.D - Make sure that every function in Phobos has a public example
- Seb (39/39) Jul 20 2017 Examples are a great way to explore functions on Phobos. They
- Wolfram W. (40/40) Jul 24 2017 Hi Seb,
- Seb (52/89) Jul 24 2017 Some tests are quite old and AFAICT documented examples weren't
Examples are a great way to explore functions on Phobos. They provide an intuitive, playful alternative to reading the docs - especially now that they are interactive [1]. They are so important that people even report bugs if they don't exist [2]. However, Phobos still doesn't come with a (public) unittest for every function. So, do you have ten minutes to improve Phobos and "unittestify" a module? This doesn't require in-depth knowledge of Phobos and is a great way to get started with Phobos's codebase. The workflow is rather easy: 1) Pick a module from the blacklist [3] 2) Remove it from the blacklist 3) Run DScanner: `make -f posix.mak dscanner` 4) Add public unittest to the missing places (A public unittest is marked with a /// comment) 5) Run the test of the module to see if your new examples are correct: With the newest compiler: dmd -unittest -run std/foo.d Otherwise: make -f posix.mak std/foo.test 6) Create a branch, commit your work & send a pull request 7) GOTO easy; I went ahead and set a good example: https://github.com/dlang/phobos/pull/5581 Of course, each module can be a separate PR as well. Let me know if you have questions or run into troubles. For example, on Ubuntu 16.10 and Arch Linux, you need to use PIC=1 (e.g. make -f posix.mak dscanner PIC=1) to compile the examples (at least until [4] is merged). Current blacklist [3] --------------------- has_public_example="-etc.c.curl,-etc.c.sqlite3,-etc.c.zlib,-std.bitmanip,-std.complex,-std.concurrency,-std.container.array,-std.container.dlist,-std.container.rbtree,-std.container.slist,-std.conv,-std.csv,-std.datetime,-std.datetime.date,-std.datetime.interval,-std.datetime.stopwatch,-std.datetime.systime,-std.datetime.timezone,-std.demangle,-std.digest,-std.digest.hmac,-std.digest.murmurhash,-std.digest.sha,-std.encoding,-std.exception,-std.experimental.allocator,-std.experimental.allocator.building_blocks.allocator_list,-std.experimental.allocator.building_blocks.bitmapped_block,-std.experimental.allocator.building_blocks.fallback_allocator,-std.experimental.allocator.building_blocks.free_list,-std.experimental.allocator.building_blocks.free_tree,-std.experimental.allocator.building_blocks.null_allocator,-std.experimental.allocator.building_blocks.stats_collector,-std.experimental.allocator.common,-std.experimental.allocator.mmap_allocator,-std.experimental.allocator.typed,-std.experimental.checkedint,-std.experimental.logger.core,-std.experimental.logger.filelogger,-std.experimental.logger.multilogger,-std.experimental.typecons,-std.file,-std.format,-std.getopt,-std.internal.math.biguintcore,-std.internal.math.biguintnoasm,-std.internal.math.errorfunction,-std.internal.math.gammafunction,-std.internal.scopebuffer,-std.internal.test.dummyrange,-std.json,-std.math,-std.mathspecial,-std.mmfile,-std.net.curl,-std.net.isemail,-std.numeric,-std.outbuffer,-std.parallelism,-std.path,-std.process,-std.random,-std.range,-std.range.interfaces,-std.range.primitives,-std.regex,-std.regex.internal.ir,-std.socket,-std.stdio,-std.string,-std.traits,-std.typecons,-std.uni,-std.uri,-std.utf,-std.uuid,-std.variant,-std.xml,-std.zip,-std.zlib" [1] https://dlang.org/blog/2017/03/08/editable-and-runnable-doc-examples-on-dlang-org [2] https://issues.dlang.org/show_bug.cgi?id=17670 [3] https://github.com/dlang/phobos/blob/master/.dscanner.ini#L123 [4] https://github.com/dlang/phobos/pull/5586
Jul 20 2017
Hi Seb, thanks for the instructions. As it seems, there's actually quite a lacking in public examples for some modules, and I'd like to help with that. After pulling, scanning, and sifting through the Phobos source however, I have some questions I'd like to clarify, just to be on the safe side. Firstly, it seems that some functions already have an expressive and concise undocumented unit test (Like 'exists' in file.d, at line 1570; or 'urlEncode' in uri.d, at line 399). I see nothing wrong with also using these unit tests as examples. However, I am a bit hesitant to do so, considering these tests have been here for seemingly a long time, without being marked for documentation... so there's probably a reason for that? Secondly, especially concerning the math module--should really *every* function have an example? Most notably, examples for all the trigonometric functions and their relatives like 'acos', 'sinh, 'tan', etc. would be nearly the same and might not be that helpful, since their usage is straightforward and their results are mathematically defined anyway. 'assert(acos(1) == 0)' as a possible example seems a bit superfluous to me. Thirdly, some functions already have public examples in their comments, rather than in documented unit tests (for example 'getAddressInfo' in socket.d, at line 900). Since D-Scanner doesn't scan any comments and also for the sake of consistency, would it make sense to extract these "comment examples" and put them into documented unit tests? Lastly, I have seen that 'dirName' in the path module has a documented unit test at line 615. While D-Scanner does not report the absence of an example for the function, there is no example in the HTML documentation (see http://dlang.org/phobos/std_path.html#dirName). Could this be a bug in the HTML generator, or is there something wrong with how the unit test is written? Sorry for posting such a wall of text. As a (hopefully soon to be) first-time contributor I want to make sure not to clog the PRs with sub-par changes. Thanks, W (PS: Line numbers refer to the current commit, e8ef731e48e6ca7a13ad813da1c3ea1aeae93d01)
Jul 24 2017
On Monday, 24 July 2017 at 10:24:48 UTC, Wolfram W. wrote:Hi Seb, thanks for the instructions. As it seems, there's actually quite a lacking in public examples for some modules, and I'd like to help with that. After pulling, scanning, and sifting through the Phobos source however, I have some questions I'd like to clarify, just to be on the safe side. Firstly, it seems that some functions already have an expressive and concise undocumented unit test (Like 'exists' in file.d, at line 1570; or 'urlEncode' in uri.d, at line 399). I see nothing wrong with also using these unit tests as examples. However, I am a bit hesitant to do so, considering these tests have been here for seemingly a long time, without being marked for documentation... so there's probably a reason for that?Some tests are quite old and AFAICT documented examples weren't popular back then (or maybe not even supported). There's nothing wrong with exposing these tests. In general, public unittests should ideally - be easy to read and understand - don't expose implementation details - don't use private functions (we already check this automatically) Both tests that you references look short & concise to me. A bit of background: deleteme isn't publicly documented, but used by so many public tests that we can't remove it anymore, but probably will document it in the future [1]. [1] https://github.com/dlang/phobos/pull/5400Secondly, especially concerning the math module--should really *every* function have an example? Most notably, examples for all the trigonometric functions and their relatives like 'acos', 'sinh, 'tan', etc. would be nearly the same and might not be that helpful, since their usage is straightforward and their results are mathematically defined anyway. 'assert(acos(1) == 0)' as a possible example seems a bit superfluous to me.Hehe, I wouldn't focus on std.math. There's value in providing an example for every function (e.g. "intuitive" overview of the special cases (e.g. `tan(double.nan)`) and being able to use it as a REPL, i.e. hit "Edit" on the example and check your value), but I agree that it's definitely not as helpful as other modules. -> Prioritize If, at a distant point in the future, Phobos is so nicely documented that every module except for std.math* has public examples, adding examples to std.math for consistency could be a final step.Thirdly, some functions already have public examples in their comments, rather than in documented unit tests (for example 'getAddressInfo' in socket.d, at line 900). Since D-Scanner doesn't scan any comments and also for the sake of consistency, would it make sense to extract these "comment examples" and put them into documented unit tests?Commented examples should be converted to unittest examples whenever possible. There have been many incidents in the past when these documentation examples didn't work anymore. In the case of `getAddressInfo` it's a bit problematic, because the testsuite should be runnable without internet, but using sth. like `scope(failure)` and thus making the test "softfail" should work.Lastly, I have seen that 'dirName' in the path module has a documented unit test at line 615. While D-Scanner does not report the absence of an example for the function, there is no example in the HTML documentation (see http://dlang.org/phobos/std_path.html#dirName). Could this be a bug in the HTML generator, or is there something wrong with how the unit test is written?Good catch! Imho this is a bug / missing feature in Ddoc and I have opened an enhancement request [2]. Note that Ddox (a replacement of builtin Ddoc) [3], parses this correctly: https://dlang.org/library/std/path/dir_name.html I'm not sure how easy / workable changing the behavior of Ddoc is - otherwise we might want to look into mimicking the Ddoc behavior at the Dscanner check. I have opened an issue there as well, s.t. it's not forgotten [4]. Thanks again for bringing this up! [2] https://issues.dlang.org/show_bug.cgi?id=17678 [3] https://github.com/rejectedsoftware/ddox [4] https://github.com/dlang-community/D-Scanner/issues/500Sorry for posting such a wall of text. As a (hopefully soon to be) first-time contributor I want to make sure not to clog the PRs with sub-par changes.No worries, I hope my answers help. Otherwise don't hesitate to ask again ;-) Also please don't be afraid to submit a "sub-par PR" - you won't be killed for it. Just be careful to avoid a huge PR - multiple PRs with a small diff will be pulled quicker.
Jul 24 2017