darcs

Patch 430 Bake unit tests into the modules they are testing.

Title Bake unit tests into the modules they are testing.
Superseder Nosy List kowey
Related Issues
Status rejected Assigned To
Milestone

Created on 2010-10-19.23:06:16 by kowey, last changed 2011-05-10.17:35:57 by darcswatch. Tracked on DarcsWatch.

Files
File name Status Uploaded Type Edit Remove
bake-unit-tests-into-the-modules-they-are-testing_.dpatch kowey, 2010-10-19.23:06:16 text/x-darcs-patch
unnamed kowey, 2010-10-19.23:06:16
See mailing list archives for discussion on individual patches.
Messages
msg12776 (view) Author: kowey Date: 2010-10-19.23:06:16
1 patch for repository http://darcs.net/screened:

I'm not going to screen it because I'm not sure if it's actually a good
idea or not.

Pro:
 - no extra hierarchy
 - sort of a sense/hint that all modules should have a test suite
   (minimal bureaucracy)

Eh:
 - but then Darcs.Patch seems to need one anyway

Con:
 - modules importing things they don't need (because only needed by testing?)

Tue Oct 19 17:34:02 BST 2010  Eric Kow <kowey@darcs.net>
  * Bake unit tests into the modules they are testing.
  For tests that are particularly complex/extensive/multi-module, we
  just use a .Test subtree.
Attachments
msg12880 (view) Author: ganesh Date: 2010-11-02.22:37:11
I'm somewhat against this change. Most significantly, it introduces a 
QuickCheck dependency into our main codebase, and this could be 
problematic for users of the library because of the 
QuickCheck1/QuickCheck2 dependency fight.

Also, there's a design decision about our tests to make, namely whether 
they should have access to the internals of the thing they're testing or 
not (white-box versus black-box). My current feeling about the Patch 
hierarchy is that black-box testing is the best approach, at least for 
the bulk of the tests. I'd therefore like to keep the tests out of the 
source files they refer to and cut down on/eliminate the symbols that 
are exported just for test purposes.

Finally, I'm currently knee deep in reorganising Darcs.Patch and this 
bundle would disrupt me rather, though it's not a strong reason if the 
consensus is in favour of it.
msg12894 (view) Author: kowey Date: 2010-11-03.13:06:47
So original submitter (me) is a bit nervous about this, and a reviewer
too, so maybe we should just reject this for now and come back to it
when the re-org dust has settled.

Note-taking for posterity

Pro:
 - ability to test non-exported functions

Con:
 - encourages whitebox rather than blackbox testing

Ganesh's preference shows us a link between these two: if we think of
exporting things just for testing as being a code smell that could
perhaps be eliminated with some re-thinking, maybe the Pro isn't such a
Pro after all.

When the dust settles, one alternative form of juggling I might consider
is renaming Darcs.Test.X to Test.Darcs.X.  This seems a bit silly, I
realise.  The only goal behind this proposal is to make it possible to
have a parallel hierarchy including non-Darcs modules, so Test.Lcs and
Test.ByteStringUtils for example.  But maybe the testing stuff is
transparent enough as it is.
msg12941 (view) Author: tux_rocker Date: 2010-11-08.16:54:28
Op woensdag 03 november 2010 14:06 schreef Eric Kow:
> So original submitter (me) is a bit nervous about this, and a reviewer
> too, so maybe we should just reject this for now and come back to it
> when the re-org dust has settled.
> 
> Note-taking for posterity
> 
> Pro:
>  - ability to test non-exported functions
> 
> Con:
>  - encourages whitebox rather than blackbox testing

And adds a dependency on QuickCheck and HUnit and all that stuff. Which may
cause dependency problems for people who just want to use darcs. 

So I vote against merging tests into the modules they test.

> When the dust settles, one alternative form of juggling I might consider
> is renaming Darcs.Test.X to Test.Darcs.X.  This seems a bit silly, I
> realise.  The only goal behind this proposal is to make it possible to
> have a parallel hierarchy including non-Darcs modules, so Test.Lcs and
> Test.ByteStringUtils for example.  But maybe the testing stuff is
> transparent enough as it is.

I believe Test.* is used for libraries that provide testing tools, like
Test.QuickCheck, Test.HUnit, Test.Framework etc. But darcs is not a testing
tool. Moreover, these test modules are a part of darcs. So I think Darcs.Test
is really more appropriate.
msg13067 (view) Author: kowey Date: 2010-11-15.15:03:36
On Mon, Nov 08, 2010 at 16:54:28 +0000, Reinier Lamers wrote:
> I believe Test.* is used for libraries that provide testing tools, like
> Test.QuickCheck, Test.HUnit, Test.Framework etc. But darcs is not a testing
> tool. Moreover, these test modules are a part of darcs. So I think Darcs.Test
> is really more appropriate.

Oh. Yeah, the Darcs.Test.Misc approach (status quo) is fine

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, try +44 (0)1273 64 2905 or
xmpp:kowey@jabber.fr (Jabber or Google Talk only)
History
Date User Action Args
2010-10-19 23:06:16koweycreate
2010-10-19 23:09:36darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-1dbc2a8d2fa31b5d6168e8a99e9e73e0ceeec17e
2010-11-02 22:37:11ganeshsetmessages: + msg12880
2010-11-03 13:06:47koweysetstatus: needs-screening -> rejected
messages: + msg12894
2010-11-08 16:54:28tux_rockersetmessages: + msg12941
2010-11-15 15:03:36koweysetmessages: + msg13067
2011-05-10 17:35:57darcswatchsetdarcswatchurl: http://darcswatch.nomeata.de/repo_http:__darcs.net_.html#bundle-1dbc2a8d2fa31b5d6168e8a99e9e73e0ceeec17e -> http://darcswatch.nomeata.de/repo_http:__darcs.net_reviewed.html#bundle-1dbc2a8d2fa31b5d6168e8a99e9e73e0ceeec17e