[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
On white-box tests
From: |
Ludovic Courtès |
Subject: |
On white-box tests |
Date: |
Wed, 19 Aug 2009 10:38:07 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
Hello!
Just a note that I've been meaning to send for some time.
"Michael Gran" <address@hidden> 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'.
- On white-box tests,
Ludovic Courtès <=