Hi all,
I went through Istvan's refactored tests this evening, and I have a
bunch of thoughts, notes and comments. I'm not sure how to organize
them so apologies in advance if they're disorganized ;).
This is re Istvan's github branch,
http://github.com/ialbert/pygr-psu/tree/master
Let me say straight off that I think this refactoring is a Good Thing
and has simplified many aspects of the tests. If I seem nitpicky below,
it's because that's what I'm paid to do.
Note that I did not run either the MySQL or the XML-RPC tests.
========
I first looked at code coverage to see if there were any dramatic
changes there; that would be a red flag. Without doing an actual
subtraction, I can't be sure, but it looks like the code coverage has
stayed the same:
Istvan's branch: http://lyorn.idyll.org/~t/transfer/coverage/
pygr head: http://lyorn.idyll.org/~t/transfer/coverage2/
========
Right now all of the test scripts and the test utilities are in one
directory. What do you think of making a 'testlib' package, or a
subdirectory that contains all of the test utilities?
========
Test running is a little bit clumsy at the moment when you want to run
all of the tests. The output is verbose, and it's hard to see failing
tests as they scroll by.
There may be two (compatible) options here.
One is to provide a single unittest test runner that gathers all of the
tests together into one test suite. I'm pretty sure unittest can do
this, although I'm not positive.
The other is to make sure that nose can gather all of the tests
properly. If we can provide a single unittest test runner, that would
be usable by nose; otherwise there may be some additional hackery to be
done. I don't think there's a downside to making things
nose-executable, as long as it's relatively simple and keeps the
unittest compatibility. This could eventually replace runtest, which is
nice but is a bit superfluous in the face of community-supported
frameworks like nose.
(I think runtest would be good to keep as long as it stays simple.)
========
The temp directory should probably be nuked each time the tests are run.
The second time I ran the tests I got a prompt complaining that a file
already existed & did I want to overwrite it?
========
Other issues:
There are a number of deviations from PEP 8 style in the new test code,
compounding some of the original deviations ;). The two most notable
things are long lines (len > 80) and extra spacing inside of
parantheses,
fn( arg1, arg2 )
There are also some 'import' statements inside of functions, which is
generally frowned upon -- see blast_test.py.
These should all be fixed. I'm happy to do that on my second code
review as long as everyone is OK with it.
=========
Also, a pseudo-patch for minor things I picked up along the way is
attached. This is just misspellings etc.
So, that's my first run through. Comments & thoughts welcome.
cheers,
--titus
--
C. Titus Brown, [email protected]
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"pygr-dev" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/pygr-dev?hl=en
-~----------~----~----~----~------~----~------~--~---
index 9baaa54..2f68621 100644
--- a/tests/blast_test.py
+++ b/tests/blast_test.py
@@ -10,6 +10,7 @@ class Blast_Test(unittest.TestCase):
tempdir = testutil.TempDir( 'blast-test' )
testutil.change_pygrdatapath( tempdir.path )
+ # @CTB?
# where do these come from?
#self.genomes = ['.'.join(x.split('.')[-2:]) for x in pygr.Data.dir('Bi
#available_exons = [x for x in pygr.Data.dir('Bio.Annotation.ASAP2') if
diff --git a/tests/pathfix.py b/tests/pathfix.py
index 8859b54..c69de86 100644
--- a/tests/pathfix.py
+++ b/tests/pathfix.py
@@ -1,6 +1,7 @@
"""
The sole purpose of this module is to alter the sys.path upon
-import in such a way to get pygr from the source directory rather than
+import in such a way to get pygr from the source directory rather than
+...@ctb finish comment?
See the README.txt file for details on how to change the behavior.
diff --git a/tests/sqlsequence_test.py b/tests/sqlsequence_test.py
index 7cac150..e795810 100644
--- a/tests/sqlsequence_test.py
+++ b/tests/sqlsequence_test.py
@@ -2,6 +2,8 @@ import unittest, string
import pathfix, testutil, logger
from pygr import sqlgraph, seqdb, classutil
+# @CTB 'SQLSequece' => SQLSequence
+
class SQLSequence_Test( unittest.TestCase ):
'''Basic SQL sequence class tests
