From 4bf1a63d5623d93774b8bb364eace06804394b4e Mon Sep 17 00:00:00 2001 From: Matthias Felleisen Date: Fri, 5 Aug 2011 16:56:59 -0400 Subject: [PATCH] [Style] testing, comments, some reorg; Vincent's comments --- .../scribblings/style/branch-and-commit.scrbl | 4 +- collects/scribblings/style/constructs.scrbl | 54 ++++++++++---- .../style/correct-maintain-speed.scrbl | 52 +------------- collects/scribblings/style/style.scrbl | 3 +- collects/scribblings/style/testing.scrbl | 71 +++++++++++++++++++ collects/scribblings/style/textual.scrbl | 31 +++++--- collects/scribblings/style/todo.scrbl | 11 --- collects/scribblings/style/unit.scrbl | 2 +- 8 files changed, 142 insertions(+), 86 deletions(-) create mode 100644 collects/scribblings/style/testing.scrbl diff --git a/collects/scribblings/style/branch-and-commit.scrbl b/collects/scribblings/style/branch-and-commit.scrbl index 83ae3c2042..84a32538aa 100644 --- a/collects/scribblings/style/branch-and-commit.scrbl +++ b/collects/scribblings/style/branch-and-commit.scrbl @@ -30,7 +30,9 @@ act in this context. Write meaningful commit messages. The first line (say 80 chars) should provide a concise summary of the commit. If the message must be longer, edit the rest of the message in your text editor and leave a blank line - between the summary line and the rest of the message. + between the summary line and the rest of the message. The message for bug + report fixes should contain ``Close PR NNNNN'' so that bug reports are + automatically closed. To avoid 'merge commits', update your repository with @tt{git --rebase pull}. diff --git a/collects/scribblings/style/constructs.scrbl b/collects/scribblings/style/constructs.scrbl index a093210b29..59ed51a850 100644 --- a/collects/scribblings/style/constructs.scrbl +++ b/collects/scribblings/style/constructs.scrbl @@ -20,12 +20,35 @@ Seasoned Schemers, not necessarily Racketeers, also use triple and quadruple semicolons. This is considered a courtesy to distinguish file headers from section headers. +In addition to ``;'', we have two other mechanisms for commenting code: + ``#|...|#'' for blocks and ``#;'' to comment out an expression. + +In rare cases there might be a more substantial piece of text or code, and + in that case the block comment style can be more convenient than + semicolons. Another use of block comments is for code samples to be + copied and pasted into a file. Either way, block comments are not used + for single-line comments. + +Expression comments---``#;''---apply to the following S-expression. This + makes them a useful tool for debugging. They can even be composed in + interesting ways with other comments, for example, ``#;#;'' will comment + two expressions, and a line with just ``;#;'' gives you a single-character + ``toggle'' for the expression that starts on the next line. But on the + flip side, lots tools don't process them properly---treating them instead + as a ``#'' followed by a commented line. For example, in DrRacket + S-expression comments are ignored when it comes to syntax coloring, which + makes it easy to miss them. In Emacs, the commented text is colored like a + comment and treated as text, which makes it difficult to edit as code. + The bottom line here is that ``#;'' comments are useful for debugging, but + try to avoid leaving them in committed code. If you really want to use + ``#;'', clarify their use with a line comment (``;''). + @; ----------------------------------------------------------------------------- @section{Definitions} Racket comes with quite a few definitional constructs, including @scheme[let], @scheme[let*], @scheme[letrec], and @scheme[define]. Except -for the last one, definitional construct increase the indentation level. +for the last one, definitional constructs increase the indentation level. Therefore, favor @scheme[define] when feasible. @compare[ @@ -90,23 +113,25 @@ prefer them over @scheme[if]. racket (cond - [(empty? l) true] + [(empty? l) #false] [else (define f (fir l)) (define r (rest l)) - (and (flat-rate f) - (curved f (chk r)))]) + (if (discounted? f) + (discount-rate f) + (curved f (chk r)))]) ] @racketmod[#:file @tt{bad} racket (if (empty? l) - true + #false (let ([f (fir l)] [r (rest l)]) - (and (flat-rate f) - (curved f (chk r))))) + (if (discounted? f) + (discount-rate f) + (curved f (chk r))))) ] ] @@ -198,7 +223,7 @@ Even a curried function does not need @racket[lambda]. @tt{good} racket -(define ((staged-image-composition fixed-image) variable-image) +(define ((cut fx-image) image2) ...) ] @; ----------------------------------------------------------------------------- @@ -206,8 +231,8 @@ racket @tt{acceptable} racket -(define (staged-image-composition fixed-image) - (lambda (variable-image) +(define (cut fx-image) + (lambda (image2) ...)) ] ] @@ -233,7 +258,7 @@ The identity function is @racket[values]: @section{Traversals} With the availability of @racket[for/fold], @racket[for/list], - @racket[for/vector], and friends, programming with for @racket[for] loops + @racket[for/vector], and friends, programming with @racket[for] loops has become just as functional as programming with @racket[map] and @racket[foldr]. With @racket[for*] loops, filter, and termination clauses in the iteration specification, these loops are also far more concise than @@ -270,8 +295,11 @@ racket racket ;; [Listof X] -> Number -(define (sum-up s) - (foldr (lambda (x sum) (+ sum x)) 0 s)) +(define (sum-up alist) + (foldr (lambda (x sum) + (+ sum x)) + 0 + alist)) ;; example: (sum-up '(1 2 3)) diff --git a/collects/scribblings/style/correct-maintain-speed.scrbl b/collects/scribblings/style/correct-maintain-speed.scrbl index 7f5e531d99..930aa619f3 100644 --- a/collects/scribblings/style/correct-maintain-speed.scrbl +++ b/collects/scribblings/style/correct-maintain-speed.scrbl @@ -23,7 +23,7 @@ This section explains these three points as far as the Racket code base is code base. @; ----------------------------------------------------------------------------- -@section[#:tag "correctness"]{Correctness} +@section[#:tag "correctness"]{Correctness and Testing} @nested[#:style 'inset]{@italic{I have bug reports, therefore I exist.} -- Matthias, watching Matthew, Robby, Shriram and others create the original code base} @@ -45,54 +45,8 @@ We ensure this basic level of correctness with large test suites. Our test DrRacket comes with an automatic GUI player that explores its functionality. -Most tests suites live in @tt{collects/tests/} in the PLT - repository. @margin-note*{Due to historical reasons, a few collections - come with their own local test suites.} These test suites suggest a - culture of testing per tool or language. If you add a new collection, - create a new test suite in the @tt{tests} collection. - -Run the test suites before you commit. After you commit, watch for and - read(!) @hyperlink["http://drdr.racket-lang.org/"]{DrDr}'s emails. Do - @emph{not} ignore them. If you have a tests that regularly fails, consider - splitting your test directory into two parts: @tt{success} and - @tt{failure}. The former is for tests that should succeed now, and the - latter is for tests that are currently intended to fail. See the - @hyperlink["https://github.com/plt/racket/tree/master/collects/tests/typed-scheme"]{Typed - Racket testing arrangement} for an example. - -When you debug an existing piece of code, formulate a test case first. Put - it into the test suite for the component so the mistake will never be - accidentally re-introduced and add a note that points to the problem - report. Second, modify the code to fix the mistake. Do this second to be - sure you didn't introduce a mistake in your tests; it is all too easy to - think you have fixed a mistake when in reality your new test just doesn't - properly reveal the old mistake. Third, re-run the test suite to ensure - that the mistake is fixed and no existing tests fail. - -If there is no test suite and you aren't sure how to build one, then ask on - the developer mailing list. Perhaps people will explain why there isn't - one or they will sketch how to create one. Please don't ignore the - problem. If you cannot build a test suite, you have a few options: - -@itemlist[#:style 'ordered - - @item{Add functionality to the library to enable testing. Of course, - adding functionality means adding external documentation. Robby and - Matthew have done so for the GUI library, and there is now a large - automated test suite for DrRacket. So even GUI programs can come with - extended test suites.} - - @item{Add an end-to-end test that may have to be verified by a human. - For example, it might be hard to test Slideshow, so you could create a - slide set and describe what it should look like so future maintainers to - verify when @emph{they} make changes. Consider this the @emph{last and - least desirable} option, however.} -] -@; - The lack of tests for some collection will not disappear overnight. But if - we all contribute a little bit, we will eventually expand the test suites - to cover the entire code base, and future generations of maintainers will - be grateful. +For details on testing in the context of the Racket code base, see + @secref{testing}. @; ----------------------------------------------------------------------------- @section{Maintenance} diff --git a/collects/scribblings/style/style.scrbl b/collects/scribblings/style/style.scrbl index a20ada9602..30bab23647 100644 --- a/collects/scribblings/style/style.scrbl +++ b/collects/scribblings/style/style.scrbl @@ -56,10 +56,11 @@ Also, look over the commit messages. If you see problems with the code @; ----------------------------------------------------------------------------- @include-section{correct-maintain-speed.scrbl} -@include-section{some-performance.scrbl} +@include-section{testing.scrbl} @include-section{unit.scrbl} @include-section{constructs.scrbl} @include-section{textual.scrbl} +@include-section{some-performance.scrbl} @include-section{branch-and-commit.scrbl} @include-section{acknowledgment.scrbl} diff --git a/collects/scribblings/style/testing.scrbl b/collects/scribblings/style/testing.scrbl new file mode 100644 index 0000000000..ac02fb8bad --- /dev/null +++ b/collects/scribblings/style/testing.scrbl @@ -0,0 +1,71 @@ +#lang scribble/base + +@(require "shared.rkt") + +@title[#:tag "testing"]{Testing} + +@; ----------------------------------------------------------------------------- +@section[#:tag "test-suite"]{Test Suites} + +Most of our collections come with test suites. These tests suites tend to + live in @tt{collects/tests/} in the PLT repository, though due to + historical reasons, a few collections come with their own local test + suites. If you add a new collection, create a new test suite in the + @tt{tests} collection. + +Run the test suites before you commit. To facilitate testing, we urge you + to add a @tt{TESTME.txt} file to your collections. Ideally, you may also + wish to have a file in this directory that runs the basic tests. See the + @hyperlink["https://github.com/plt/racket/tree/master/collects/2htdp/"]{2htdp}, + which is one of the collections with own testing style. The file should + describe where the tests are located, how to run thee tests, and what to + look for in terms of successes and failures. These files are necessary + because different collections have different needs for testing, and + testing evolved in many different ways in our history. + +After you commit, watch for and read(!) + @hyperlink["http://drdr.racket-lang.org/"]{DrDr}'s emails. Do @emph{not} + ignore them. If you have tests that are known to fail and fixing this + requires a lot of work, consider splitting your test directory into two + parts: @tt{success} and @tt{failure}. The former is for tests that should + succeed now, and the latter is for tests that are currently expected to + fail. See the + @hyperlink["https://github.com/plt/racket/tree/master/collects/tests/typed-scheme"]{Typed + Racket testing arrangement} for an example. + +@; ----------------------------------------------------------------------------- +@section[#:tag "test-bang"]{Always Test!} + +When you debug an existing piece of code, formulate a test case first. Put + it into the test suite for the component so the mistake will never be + accidentally re-introduced and add a note that points to the problem + report. Second, modify the code to fix the mistake. Do this second to be + sure you didn't introduce a mistake in your tests; it is all too easy to + think you have fixed a mistake when in reality your new test just doesn't + properly reveal the old mistake. Third, re-run the test suite to ensure + that the mistake is fixed and no existing tests fail. + +If there is no test suite and you aren't sure how to build one, then ask on + the developer mailing list. Perhaps people will explain why there isn't + one or they will sketch how to create one. Please don't ignore the + problem. If you cannot build a test suite, you have a few options: + +@itemlist[#:style 'ordered + + @item{Add functionality to the library to enable testing. Of course, + adding functionality means adding external documentation. Robby and + Matthew have done so for the GUI library, and there is now a large + automated test suite for DrRacket. So even GUI programs can come with + extended test suites.} + + @item{Add an end-to-end test that may have to be verified by a human. + For example, it might be hard to test Slideshow, so you could create a + slide set and describe what it should look like so future maintainers to + verify when @emph{they} make changes. Consider this the @emph{last and + least desirable} option, however.} +] +@; + The lack of tests for some collection will not disappear overnight. But if + we all contribute a little bit, we will eventually expand the test suites + to cover the entire code base, and future generations of maintainers will + be grateful. diff --git a/collects/scribblings/style/textual.scrbl b/collects/scribblings/style/textual.scrbl index 9fe3e6df92..5557d5b3f6 100644 --- a/collects/scribblings/style/textual.scrbl +++ b/collects/scribblings/style/textual.scrbl @@ -120,6 +120,17 @@ patient and use the existing indentation tool anyway. @bold{Caveat 2}: This rule does not apply to scribble code. +@; ----------------------------------------------------------------------------- +@section{Tabs} + +Do not use tab characters in your code. Tabs make it hard to use textual + tools like git or diff effectively. To disable tabs, +@itemlist[ +@item{in DrRacket: you are all set. It doesn't insert tabs.} +@item{in Emacs: add (setq indent-tabs-mode nil) to your emacs initialization file.} +@item{in vi: set expandtab.} +] + @; ----------------------------------------------------------------------------- @section{Line Breaks} @@ -192,19 +203,18 @@ racket (code:comment #, @t{and}) -(composition img - (- width hdelta) - (- height vdelta) - bg) - +(above img + (- width hdelta) + (- height vdelta) + bg) ] @racketmod[#:file @tt{bad} racket -(composition ufo - 10 v-delta bg) +(above ufo + 10 v-delta bg) ]] @@ -281,7 +291,8 @@ traverse_forest ] @; Note that _ (the underline character) is also classified as bad - Racketeering. + Racketeering within names. It is an acceptable placeholder in syntax + patterns, match patterns, and parameters that don't matter. Another widely used convention is to @emph{prefix} a function name with the data type of the main argument. This convention generalizes the selector-style @@ -331,7 +342,7 @@ Finally, in addition to regular alphanumeric characters, Racketeers use a @row[<%> "interfaces" dc<%>] @row[^ "unit signatures" game-context^] @row["@" "units" testing-context@] - @row["#%" "kernel identifiers" #:app] + @row["#%" "kernel identifiers" #%app] ] The use of ``#%'' to prefix names from the kernel language warns readers that these identifiers are extremely special and they need to watch out @@ -350,7 +361,7 @@ may relax this constraint. @section{Spaces} -Don't pollute your code with spaces and extraneous blank lines. +Don't pollute your code with spaces at the end of lines and extraneous blank lines. @; ----------------------------------------------------------------------------- @section{End of File} diff --git a/collects/scribblings/style/todo.scrbl b/collects/scribblings/style/todo.scrbl index 6d3ee7186f..5bbdb8d8b5 100644 --- a/collects/scribblings/style/todo.scrbl +++ b/collects/scribblings/style/todo.scrbl @@ -8,12 +8,6 @@ @item{Write a section on when macros, when functions.} -@item{Write a section on how to organize test suites -- - -Robby: please review the "correctness" section (1.1) and determine what we -need to add. It turns out that the point showed up in several feedback -messages so a hand-wavy thing is probably not enough.} - @item{Write a section on how to design test cases.} @item{Write a section on how to check the stressability of your software.} @@ -22,9 +16,4 @@ messages so a hand-wavy thing is probably not enough.} @item{Do we need a discussion of life cycles that start in @tt{unstable}?} -@item{The section on comments (constructs) should be expanded to cover -S-expression comments etc. -- - -Eli: you seem to have good ideas. Please write up an explanation.} - ] diff --git a/collects/scribblings/style/unit.scrbl b/collects/scribblings/style/unit.scrbl index 2ab6f4b96b..012d8a9923 100644 --- a/collects/scribblings/style/unit.scrbl +++ b/collects/scribblings/style/unit.scrbl @@ -138,7 +138,7 @@ Avoid @racket[(provide (all-defined-out))] except for general-purpose libraries. A test suite section---if located within the module---should come at the - every end, including its specific dependencies, i.e., @racket[require] + very end, including its specific dependencies, i.e., @racket[require] specifications. @; -----------------------------------------------------------------------------