unofficial mirror of guile-devel@gnu.org 
 help / color / mirror / Atom feed
From: ludo@gnu.org (Ludovic Courtès)
To: "Michael Gran" <spk121@yahoo.com>
Cc: guile-devel@gnu.org
Subject: On white-box tests
Date: Wed, 19 Aug 2009 10:38:07 +0200	[thread overview]
Message-ID: <87d46sci6o.fsf@gnu.org> (raw)
In-Reply-To: <E1Majpl-0008Ga-T9@cvs.savannah.gnu.org> (Michael Gran's message of "Tue, 11 Aug 2009 05:23:42 +0000")

Hello!

Just a note that I've been meaning to send for some time.

"Michael Gran" <spk121@yahoo.com> writes:

> http://git.savannah.gnu.org/cgit/guile.git/commit/?id=9b0c25f6d18d5be318ea3a47fd87cf7e63b689e1

[...]

> --- a/test-suite/tests/strings.test
> +++ b/test-suite/tests/strings.test

[...]

> +;;
> +;; string internals
> +;;
> +
> +;; Some abbreviations
> +;; BMP - Basic Multilingual Plane (codepoints below U+FFFF)
> +;; SMP - Suplementary Multilingual Plane (codebpoints from U+10000 to U+1FFFF)
> +
> +(with-test-prefix "string internals"
> +
> +  (pass-if "new string starts at 1st char in stringbuf"
> +    (let ((s "abc"))
> +      (= 0 (assq-ref (%string-dump s) 'start))))
> +
> +  (pass-if "length of new string same as stringbuf"
> +    (let ((s "def"))
> +      (= (string-length s) (assq-ref (%string-dump s) 'stringbuf-length))))
> +
> +  (pass-if "contents of new string same as stringbuf"
> +    (let ((s "ghi"))
> +      (string=? s (assq-ref (%string-dump s) 'stringbuf-chars))))
> +
> +  (pass-if "writable strings are not read-only"
> +    (let ((s "zyx"))
> +      (not (assq-ref (%string-dump s) 'read-only))))
> +
> +  (pass-if "read-only strings are read-only"
> +    (let ((s (substring/read-only "zyx" 0)))
> +      (assq-ref (%string-dump s) 'read-only)))
> +
> +  (pass-if "null strings are inlined"
> +    (let ((s ""))
> +      (assq-ref (%string-dump s) 'stringbuf-inline)))

First of all, thanks for taking the time to write unit tests!

I'm not fully convinced by some of these "string internals" tests,
though.  These are "white box tests".  I believe these tests are mostly
useful when written by someone different than the one who implemented
the code under test, both of whom following a given specification.  When
someone writes both the code and the white box tests, I'm afraid they
end up writing the same code twice, just differently.  For example:

  SCM
  scm_i_substring_read_only (SCM str, size_t start, size_t end)
  {
    [...]

    return scm_double_cell (RO_STRING_TAG,  /* <--- read-only */
                            SCM_UNPACK(buf),
                            (scm_t_bits)str_start + start,
                            (scm_t_bits) end - start);
  }

and:

  (pass-if "read-only strings are read-only"
    (let ((s (substring/read-only "zyx" 0)))
      (assq-ref (%string-dump s) 'read-only)))

Thus, I think such tests don't provide much information.

Conversely, for this example, a black-box test of the public API would
have helped catch regressions and non-conformance issues:

  (pass-if-exception exception:read-only-string
    (string-set! (substring/read-only "zyx" 0) 1 #\x))

Another issue is that these tests expose a lot of the implementation
details, e.g.:

  (pass-if "short Latin-1 encoded strings are inlined"
    (let ((s "m"))
      (assq-ref (%string-dump s) 'stringbuf-inline)))

The inline/outline distinction is an implementation detail.  If we
change it (that's something we could get rid of in the BDW-GC branch,
for instance) then the tests will have to be rewritten or removed.

Conversely, black box tests can give confidence that a change in
implementation details did have any user-visible impact.

There *are* situations where the gap between the public API and the
internals is too important, and white box tests can come in handy here
(that's why `%string-dump' et al. are nice tools).  However, often, I
think it's better to have good coverage of the public API than a wealth
of "obvious" white-box tests.

What do you think?

Thanks,
Ludo'.




       reply	other threads:[~2009-08-19  8:38 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <E1Majpl-0008Ga-T9@cvs.savannah.gnu.org>
2009-08-19  8:38 ` Ludovic Courtès [this message]
2009-08-19 12:14   ` On white-box tests Mike Gran
2009-08-19 13:53     ` Ludovic Courtès
2009-08-19 14:28       ` Mike Gran
2009-08-19 14:41         ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/guile/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d46sci6o.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=guile-devel@gnu.org \
    --cc=spk121@yahoo.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).