Tasks #4066
unit tests for src/include/buffer.h
100%
Description
The tests for buffer.h are incomplete They must cover 100% of the lines of code.
joshd: loicd: if ever a class could use unit testing (and documentation), it's bufferlist. loicd: I'll add to https://github.com/ceph/ceph/blob/master/src/test/bufferlist.cc in order to improve the coverage of http://dachary.org/wp-uploads/2013/01/ceph/common/buffer.cc.gcov.html joshd: loicd: yeah
Related issues
Associated revisions
unit tests for src/common/buffer.{cc,h}
Implement unit tests covering most lines of code ( > 92% ) and all
methods as show by the output of make check-coverage :
http://dachary.org/wp-uploads/2013/03/ceph-lcov/ .
The following static constructors are implemented by opaque classes
defined in buffer.cc ( buffer::raw_char, buffer::raw_posix_aligned
etc. ). Testing the implementation of these classes is done by
variations of the calls to the static constructors.
copy(const char *c, unsigned len);
create(unsigned len);
claim_char(unsigned len, char *buf);
create_malloc(unsigned len);
claim_malloc(unsigned len, char *buf);
create_static(unsigned len, char *buf);
create_page_aligned(unsigned len);
The raw_mmap_pages class cannot be tested because it is commented out in
raw_posix_aligned. The raw_hack_aligned class is only tested under Cygwin.
The raw_posix_aligned class is not tested under Cygwin.
The unittest_bufferlist.sh script calls unittest_bufferlist with the
CEPH_BUFFER_TRACK=true environment variable to enable the code
tracking the memory usage. It cannot be done within the bufferlist.cc
file itself because it relies on the initialization of a global
variable ( buffer_track_alloc ).
When raw_posix_aligned is called on DARWIN, the data is not aligned
on CEPH_PAGE_SIZE because it calls valloc(size) which is the equivalent of
memalign(sysconf(_SC_PAGESIZE),size) and not memalign(CEPH_PAGE_SIZE,size).
For this reason the alignment test is de-activated on DARWIN.
The tests are grouped in
TEST for buffer::ptr
TEST for buffer::list::iterator
TEST for buffer::list
TEST for buffer::hash
and each method ( and all variations of the prototype ) are
included into a single TEST function.
Although most aspects of the methods are tested, including exceptions
and border cases, inconsistencies are not highlighted . For
instance
buffer::list::iterator i;
i.advance(1);
would dereference a buffer::raw NULL pointer although
buffer::ptr p;
p.wasted()
asserts instead of dereferencing the buffer::raw NULL pointer. It
would be better to always assert in case a NULL pointer is about to be
used. But this is a minor inconsistency that is probably not worth a
test.
The following buffer::list methods
ssize_t read_fd(int fd, size_t len);
int write_fd(int fd) const;
are not fully tested because the border cases cannot be reliably
reproduced. Going thru a pointer indirection when calling the ::writev
or safe_read functions would allow the test to create mockups to synthetize
the conditions for border cases.
tracker.ceph.com/issues/4066 refs #4066
Signed-off-by: Loic Dachary <loic@dachary.org>
History
#1 Updated by Loïc Dachary about 11 years ago
#2 Updated by Loïc Dachary about 11 years ago
- Status changed from New to Fix Under Review
- % Done changed from 0 to 90
#4 Updated by Loïc Dachary about 11 years ago
- Status changed from Fix Under Review to Resolved
- % Done changed from 90 to 100