Unit test for cfi_cmdset_0001.c

Unit test for cfi_cmdset_0001.c

[Jared Hulbert]

I plan on creating a unit test based on cutest.sourceforge.net for
cfi_cmdset_0001.c.
In order to stub out the various functions inside I will have to:

1. Manually strip out functions for each version.
2. Add #if''s
3. fork this file into 2-3 files.

I prefer option 3.  As it seems cleaner to me and more maintainable. 
But I want to know which option is more "commitable."   Of course the
test be made availiable, I would hope we can get the tests commited
too.  CuTest is zlib licenced.  Is that a problem?

The reason for the unit tests is to help us find bugs in the code
paths not easily tested and to enable more rapid updates for new flash
or features mtd doesn't currently support.

Thoughts, flames, jokes?

,Jared

Re: Unit test for cfi_cmdset_0001.c

[Nicolas Pitre]

On Thu, 27 Jan 2005, Jared Hulbert wrote:

> I plan on creating a unit test based on cutest.sourceforge.net for
> cfi_cmdset_0001.c.
> In order to stub out the various functions inside I will have to:
> 
> 1. Manually strip out functions for each version.
> 2. Add #if''s
> 3. fork this file into 2-3 files.
> 
> I prefer option 3.  As it seems cleaner to me and more maintainable. 
> But I want to know which option is more "commitable."   Of course the
> test be made availiable, I would hope we can get the tests commited
> too.

I'm skeptical.  How cutest is going to be of any benefit here?

> CuTest is zlib licenced.  Is that a problem?

It would if not GPL compatible.

> The reason for the unit tests is to help us find bugs in the code
> paths not easily tested and to enable more rapid updates for new flash
> or features mtd doesn't currently support.

You will have to show me that cutest can really test such code.  It's 
not just algorithmic code we're talking about but a kernel driver for 
hardware.  Hardware has subtle but random timing variations, and a 
kernel driver needs to cope with scheduling, sleeping, locking, etc.  I 
don't see cutest able to verify that proper locking against real life 
concurrent access is correctly implemented while being as efficient as 
possible for example.  IMHO it would only give you a false sense of 
confidence.


Nicolas

Re: Unit test for cfi_cmdset_0001.c

[Josh Boyer]

On Thu, 2005-01-27 at 17:20 -0800, Jared Hulbert wrote:
> I plan on creating a unit test based on cutest.sourceforge.net for
> cfi_cmdset_0001.c.

How feasible is a framework like that for things that deal with
hardware?  Are you planning on emulating flash chips somehow?

> In order to stub out the various functions inside I will have to:
> 
> 1. Manually strip out functions for each version.

Not sure what you mean.  Could be partially due to my ignorance of
cuTest, so bear with me :).

> 2. Add #if''s
> 3. fork this file into 2-3 files.
> 
> I prefer option 3.  As it seems cleaner to me and more maintainable. 

Maybe an example of what you intend here?

> But I want to know which option is more "commitable."   Of course the
> test be made availiable, I would hope we can get the tests commited
> too.  CuTest is zlib licenced.  Is that a problem?
> 
> The reason for the unit tests is to help us find bugs in the code
> paths not easily tested and to enable more rapid updates for new flash
> or features mtd doesn't currently support.
> 
> Thoughts, flames, jokes?

I'm all for a test suite for MTD/JFFS[23].  Maybe cuTest could help,
maybe not.  Artem has a NAND flash simulator.  Maybe that would help
testing as well.

josh

Re: Unit test for cfi_cmdset_0001.c

[Eric W. Biederman]

Jared Hulbert <jaredeh@gmail.com> writes:

> I plan on creating a unit test based on cutest.sourceforge.net for
> cfi_cmdset_0001.c.
> In order to stub out the various functions inside I will have to:
> 
> 1. Manually strip out functions for each version.
> 2. Add #if''s
> 3. fork this file into 2-3 files.
> 
> I prefer option 3.  As it seems cleaner to me and more maintainable. 

Why would you even need to split the file into multiple pieces to
provide test wrappers?  cutest does not seem to add that requirement.
I'm not certain what value cutest actually provides.  A test harness
that  would require the source file to be split into multiple pieces
seems to indicate a flaw in the test harness.

> The reason for the unit tests is to help us find bugs in the code
> paths not easily tested and to enable more rapid updates for new flash
> or features mtd doesn't currently support.

The hardest code paths to test are code paths for hardware that
is usable but not quite right.  How do you plan on testing that?

Which features for flash chips are you thinking about supporting that
are not currently supported? 

Eric

Re: Unit test for cfi_cmdset_0001.c

[Jared Hulbert]

Wow!  Lots of sceptics!   Help!  Anybody who thinks this is an idea
that is promising please chime in. :)

Hmm, Let me try to explain a little better.

Consider cfi_cmdset_0001.c: 
cfi_intelext_write_buffers() calls cfi_intelext_write_words() and
do_write_buffer()

In order to properly unit test you'd need to stub out
cfi_intelext_write_words() and do_write_buffer().  For those that
don't understand stubbing out this means creating a phony functions to
accept input and give output based on the test condition.  This allows
you to test code paths that are improbable under nomial conditions or
very difficult to test with other methods.  But if the
do_write_buffer() is compiled into the same .o as
cfi_intelext_write_buffers() I'm not sure how to link to my stub
do_write_buffer() instead of the real one.  That is the problem that
drives my original question about #if's, forking the files, etc.

Because there are macros between this driver and any real hardware it
should be rather easy to test various states and see at least the
sequence of events the hardware will see.  This allow you to test for
hardware that does not exist or is not really worth it.  Certianly it
is not a magic bullet, it is only as good as your tests, and doesn't
find timing bugs or bugs related to multitasking etc.

I did it with a file system at work it was very useful.

,JAred

Re: Unit test for cfi_cmdset_0001.c

[Nicolas Pitre]

On Fri, 28 Jan 2005, Jared Hulbert wrote:

> Consider cfi_cmdset_0001.c: 
> cfi_intelext_write_buffers() calls cfi_intelext_write_words() and
> do_write_buffer()
> 
> In order to properly unit test you'd need to stub out
> cfi_intelext_write_words() and do_write_buffer().  For those that
> don't understand stubbing out this means creating a phony functions to
> accept input and give output based on the test condition.  This allows
> you to test code paths that are improbable under nomial conditions or
> very difficult to test with other methods.  But if the
> do_write_buffer() is compiled into the same .o as
> cfi_intelext_write_buffers() I'm not sure how to link to my stub
> do_write_buffer() instead of the real one.  That is the problem that
> drives my original question about #if's, forking the files, etc.

I can see that.  But in this particular case I think it would be better 
for code maintainability, and more importantly upstream code acceptance, 
that the code remains as is and that you write a small perl script to 
split it up automatically.  The test tools could be committed to the MTD 
CVS but they would never see the mainline tree.

> Because there are macros between this driver and any real hardware it
> should be rather easy to test various states and see at least the
> sequence of events the hardware will see.  This allow you to test for
> hardware that does not exist or is not really worth it.  Certianly it
> is not a magic bullet, it is only as good as your tests, and doesn't
> find timing bugs or bugs related to multitasking etc.

That's why I'm not sold to the idea since that particular driver sees 
its core complexity in the locking, scheduling and concurrent stomping 
on the state machine.  I don't think synthetic tests can replace careful 
inspection and perfect understanding of the code.  Static analysis tools 
like sparse should help with locking correctness though.  But I'm always 
open to be shown otherwise!

> I did it with a file system at work it was very useful.

For filesystems I can imagine.  It has many more conditions to deal with 
and making sure all of them are tested can be tedious.  But 
purely writing to 
flash is not that complex (or is it?) compared to all the needed code 
above that dealing with concurrency.

I remember the first Linux NOR flash driver I wrote in 1997: pure char 
device, no MTD layer, no reentrancy, no scheduling, just the pure flash 
read/write.  It was in the order of 20 to 30 lines of code.  I made it a 
block device with about 20 additional lines.  It even was able to deal 
with flash errors!  More than that like what we have today is 
performance stuff.


Nicolas

Re: Unit test for cfi_cmdset_0001.c

[Jared Hulbert]

> I can see that.  But in this particular case I think it would be better
> for code maintainability, and more importantly upstream code acceptance,
> that the code remains as is and that you write a small perl script to
> split it up automatically.

Right, #1 it is then.  For now that is, until ya'll see how niffty the
test tools are.

>  The test tools could be committed to the MTD
> CVS but they would never see the mainline tree.

I couldn't, wouldn't, shouldn't ask for more.

> That's why I'm not sold to the idea since that particular driver sees
> its core complexity in the locking, scheduling and concurrent stomping
> on the state machine.  I don't think synthetic tests can replace careful
> inspection and perfect understanding of the code.  Static analysis tools
> like sparse should help with locking correctness though.  But I'm always
> open to be shown otherwise!

I think you'll be surprised how much of that locking and concurrency
we can test.

> For filesystems I can imagine.  It has many more conditions to deal with
> and making sure all of them are tested can be tedious.  But
> purely writing to
> flash is not that complex (or is it?) compared to all the needed code
> above that dealing with concurrency.
> 
> I remember the first Linux NOR flash driver I wrote in 1997: pure char
> device, no MTD layer, no reentrancy, no scheduling, just the pure flash
> read/write.  It was in the order of 20 to 30 lines of code.  I made it a
> block device with about 20 additional lines.  It even was able to deal
> with flash errors!  More than that like what we have today is
> performance stuff.

Anything paragraph about coding that starts with 'I remember' is pure gold!

I don't know.  The drivers are not that simple.  Lots of obscure cases
and states.  I can say this much, almost every bug I've seen would be
have been found by a unit test.  Of course this is almost as much
because of the process of creating the tests as actually running them.
 Running the tests will make sure you don't reintroduce bugs you found
before as the code evolves.