[PATCH] Add test suite function
John Wright
jsw at debian.org
Wed Sep 22 06:18:00 UTC 2010
Hi Jelmer,
On Mon, Sep 20, 2010 at 10:34:24AM -0700, Jelmer Vernooij wrote:
> These two simple patches add a test_suite() function for python-debian
> that can be used with custom Python test runners and change debian/rules
> to call that function rather than calling all the test suites
> individually.
>
> I've also made the tests cope with running from a cwd that does not
> contain the test data.
Thanks for this! I have some nitpicks, all optional.
First patch:
> === modified file 'lib/debian/deb822.py'
> --- lib/debian/deb822.py 2010-08-01 08:06:42 +0000
> +++ lib/debian/deb822.py 2010-09-17 15:47:53 +0000
> @@ -864,6 +864,7 @@
> self.__parsed_relations = True
> return self.__relations
>
> +
Unrelated to the rest, but I agree anyway. :)
> class _multivalued(Deb822):
> """A class with (R/W) support for multivalued fields.
>
>
> === added file 'tests/__init__.py'
> --- tests/__init__.py 1970-01-01 00:00:00 +0000
> +++ tests/__init__.py 2010-09-17 15:47:53 +0000
> @@ -0,0 +1,35 @@
> +#! /usr/bin/python
> +## vim: fileencoding=utf-8
> +
> +# Copyright (C) 2010 Jelmer Vernooij <jelmer at samba.org>
> +#
> +# This program is free software; you can redistribute it and/or
> +# modify it under the terms of the GNU General Public License
> +# as published by the Free Software Foundation; version 2 or
> +# (at your option) a later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
> +
> +import unittest
> +
> +def test_suite():
> + names = [
> + 'changelog',
> + 'deb822',
> + 'debfile',
> + 'debian_support',
> + 'debtags',
> + ]
^
I'd prefer if the closing bracket was at the caret, but it's not so
important.
> + module_names = ['tests.test_' + name for name in names]
> + result = unittest.TestSuite()
> + loader = unittest.TestLoader()
> + suite = loader.loadTestsFromNames(module_names)
> + result.addTests(suite)
> + return result
>
> === modified file 'tests/test_changelog.py'
> --- tests/test_changelog.py 2010-03-14 09:16:30 +0000
> +++ tests/test_changelog.py 2010-09-17 15:47:53 +0000
> @@ -23,17 +23,21 @@
> # Copyright 2005 Frank Lichtenheld <frank at lichtenheld.de>
> # and licensed under the same license as above.
>
> +import os
> import sys
> import unittest
>
> -sys.path.insert(0, '../lib/debian/')
> +sys.path.insert(0, '../lib/')
>
> -import changelog
> +from debian import changelog
>
> class ChangelogTests(unittest.TestCase):
>
> + def open_test_data(self, filename):
> + return open(os.path.join(os.path.dirname(__file__), filename), 'r')
> +
You implemented this (or something similar) several times. What if we
had a testing module with a base testing class, like
class BaseTestCase(unittest.TestCase):
def data_path(self, filename):
return os.path.join(os.path.dirname(__file__), filename)
def open_test_data(self, filename):
return open(self.data_path(filename))
and actual tests would be
class ChangelogTests(testing.BaseTestCase):
...
Granted, both of these methods could just as well be functions in the
testing module, and we wouldn't need classes to inherit from this base
class. But this opens the door for any future common testing framework
we might consider.
In any case, having the implementation in a single module will make it a
little easier to move the testing data into another directory if we
want, since we won't have to modify each test using test data.
> def test_create_changelog(self):
> - c = open('test_changelog').read()
> + c = self.open_test_data('test_changelog').read()
> cl = changelog.Changelog(c)
> cs = str(cl)
> clines = c.split('\n')
> @@ -43,7 +47,7 @@
> self.assertEqual(len(clines), len(cslines), "Different lengths")
>
> def test_create_changelog_single_block(self):
> - c = open('test_changelog').read()
> + c = self.open_test_data('test_changelog').read()
> cl = changelog.Changelog(c, max_blocks=1)
> cs = str(cl)
> self.assertEqual(cs,
> @@ -63,7 +67,7 @@
> """)
>
> def test_modify_changelog(self):
> - c = open('test_modify_changelog1').read()
> + c = self.open_test_data('test_modify_changelog1').read()
> cl = changelog.Changelog(c)
> cl.package = 'gnutls14'
> cl.version = '1:1.4.1-2'
> @@ -72,7 +76,7 @@
> cl.add_change(' * Add magic foo')
> cl.author = 'James Westby <jw+debian at jameswestby.net>'
> cl.date = 'Sat, 16 Jul 2008 11:11:08 -0200'
> - c = open('test_modify_changelog2').read()
> + c = self.open_test_data('test_modify_changelog2').read()
> clines = c.split('\n')
> cslines = str(cl).split('\n')
> for i in range(len(clines)):
> @@ -80,7 +84,7 @@
> self.assertEqual(len(clines), len(cslines), "Different lengths")
>
> def test_add_changelog_section(self):
> - c = open('test_modify_changelog2').read()
> + c = self.open_test_data('test_modify_changelog2').read()
> cl = changelog.Changelog(c)
> cl.new_block(package='gnutls14',
> version=changelog.Version('1:1.4.1-3'),
> @@ -95,7 +99,7 @@
> cl.add_change(' * Foo did not work, let us try bar')
> cl.add_change('')
>
> - c = open('test_modify_changelog3').read()
> + c = self.open_test_data('test_modify_changelog3').read()
> clines = c.split('\n')
> cslines = str(cl).split('\n')
> for i in range(len(clines)):
> @@ -104,12 +108,12 @@
>
> def test_strange_changelogs(self):
> """ Just opens and parses a strange changelog """
> - c = open('test_strange_changelog').read()
> + c = self.open_test_data('test_strange_changelog').read()
> cl = changelog.Changelog(c)
>
> def test_set_version_with_string(self):
> - c1 = changelog.Changelog(open('test_modify_changelog1').read())
> - c2 = changelog.Changelog(open('test_modify_changelog1').read())
> + c1 = changelog.Changelog(self.open_test_data('test_modify_changelog1').read())
> + c2 = changelog.Changelog(self.open_test_data('test_modify_changelog1').read())
>
> c1.version = '1:2.3.5-2'
> c2.version = changelog.Version('1:2.3.5-2')
> @@ -135,7 +139,7 @@
> self.assertRaises(changelog.ChangelogParseError, c2.parse_changelog, cl_no_author)
>
> def test_magic_version_properties(self):
> - c = changelog.Changelog(open('test_changelog'))
> + c = changelog.Changelog(self.open_test_data('test_changelog'))
> self.assertEqual(c.debian_version, '1')
> self.assertEqual(c.full_version, '1:1.4.1-1')
> self.assertEqual(c.upstream_version, '1.4.1')
> @@ -143,7 +147,7 @@
> self.assertEqual(str(c.version), c.full_version)
>
> def test_allow_full_stops_in_distribution(self):
> - c = changelog.Changelog(open('test_changelog_full_stops'))
> + c = changelog.Changelog(self.open_test_data('test_changelog_full_stops'))
> self.assertEqual(c.debian_version, None)
> self.assertEqual(c.full_version, '1.2.3')
> self.assertEqual(str(c.version), c.full_version)
> @@ -152,21 +156,22 @@
> # The parsing of the changelog (including the string representation)
> # should be consistent whether we give a single string, a list of
> # lines, or a file object to the Changelog initializer
> - cl_data = open('test_changelog').read()
> - c1 = changelog.Changelog(open('test_changelog'))
> + cl_data = self.open_test_data('test_changelog').read()
> + c1 = changelog.Changelog(self.open_test_data('test_changelog'))
> c2 = changelog.Changelog(cl_data)
> c3 = changelog.Changelog(cl_data.splitlines())
> for c in (c1, c2, c3):
> self.assertEqual(str(c), cl_data)
>
> def test_block_iterator(self):
> - c = changelog.Changelog(open('test_changelog'))
> + c = changelog.Changelog(self.open_test_data('test_changelog'))
> self.assertEqual(map(str, c._blocks), map(str, c))
>
> def test_len(self):
> - c = changelog.Changelog(open('test_changelog'))
> + c = changelog.Changelog(self.open_test_data('test_changelog'))
> self.assertEqual(len(c._blocks), len(c))
>
> +
> class VersionTests(unittest.TestCase):
>
> def _test_version(self, full_version, epoch, upstream, debian):
>
> === modified file 'tests/test_deb822.py'
> --- tests/test_deb822.py 2010-07-25 08:46:02 +0000
> +++ tests/test_deb822.py 2010-09-17 15:47:53 +0000
> @@ -24,9 +24,14 @@
> import warnings
> from StringIO import StringIO
>
> -sys.path.insert(0, '../lib/debian/')
> -
> -import deb822
> +sys.path.insert(0, '../lib')
> +
> +from debian import deb822
> +
> +
> +def data_path(filename):
> + return os.path.join(os.path.dirname(__file__), filename)
See above.
> +
>
> # Keep the test suite compatible with python2.3 for now
> try:
> @@ -294,6 +299,7 @@
>
>
> class TestDeb822(unittest.TestCase):
> +
> def assertWellParsed(self, deb822_, dict_):
> """Check that the given Deb822 object has the very same keys and
> values as the given dict.
> @@ -424,23 +430,26 @@
> self.assertEqual(s.getvalue(), packages_content)
>
> def test_iter_paragraphs_apt_shared_storage_packages(self):
> - self._test_iter_paragraphs("test_Packages", deb822.Packages,
> + self._test_iter_paragraphs(data_path("test_Packages"), deb822.Packages,
> use_apt_pkg=True, shared_storage=True)
> +
> def test_iter_paragraphs_apt_no_shared_storage_packages(self):
> - self._test_iter_paragraphs("test_Packages", deb822.Packages,
> + self._test_iter_paragraphs(data_path("test_Packages"), deb822.Packages,
> use_apt_pkg=True, shared_storage=False)
> +
> def test_iter_paragraphs_no_apt_no_shared_storage_packages(self):
> - self._test_iter_paragraphs("test_Packages", deb822.Packages,
> + self._test_iter_paragraphs(data_path("test_Packages"), deb822.Packages,
> use_apt_pkg=False, shared_storage=False)
>
> def test_iter_paragraphs_apt_shared_storage_sources(self):
> - self._test_iter_paragraphs("test_Sources", deb822.Sources,
> + self._test_iter_paragraphs(data_path("test_Sources"), deb822.Sources,
> use_apt_pkg=True, shared_storage=True)
> +
> def test_iter_paragraphs_apt_no_shared_storage_sources(self):
> - self._test_iter_paragraphs("test_Sources", deb822.Sources,
> + self._test_iter_paragraphs(data_path("test_Sources"), deb822.Sources,
> use_apt_pkg=True, shared_storage=False)
> def test_iter_paragraphs_no_apt_no_shared_storage_sources(self):
> - self._test_iter_paragraphs("test_Sources", deb822.Sources,
> + self._test_iter_paragraphs(data_path("test_Sources"), deb822.Sources,
> use_apt_pkg=False, shared_storage=False)
>
> def test_parser_empty_input(self):
> @@ -651,11 +660,11 @@
> objects = []
> objects.append(deb822.Deb822(UNPARSED_PACKAGE))
> objects.append(deb822.Deb822(CHANGES_FILE))
> - objects.extend(deb822.Deb822.iter_paragraphs(file('test_Packages')))
> - objects.extend(deb822.Packages.iter_paragraphs(file('test_Packages')))
> - objects.extend(deb822.Deb822.iter_paragraphs(file('test_Sources')))
> + objects.extend(deb822.Deb822.iter_paragraphs(file(data_path('test_Packages'))))
> + objects.extend(deb822.Packages.iter_paragraphs(file(data_path('test_Packages'))))
> + objects.extend(deb822.Deb822.iter_paragraphs(file(data_path('test_Sources'))))
> objects.extend(deb822.Deb822.iter_paragraphs(
> - file('test_Sources.iso8859-1'), encoding="iso8859-1"))
> + file(data_path('test_Sources.iso8859-1')), encoding="iso8859-1"))
> for d in objects:
> for value in d.values():
> self.assert_(isinstance(value, unicode))
> @@ -666,16 +675,16 @@
> multi.append(deb822.Changes(CHANGES_FILE))
> multi.append(deb822.Changes(SIGNED_CHECKSUM_CHANGES_FILE
> % CHECKSUM_CHANGES_FILE))
> - multi.extend(deb822.Sources.iter_paragraphs(file('test_Sources')))
> + multi.extend(deb822.Sources.iter_paragraphs(file(data_path('test_Sources'))))
> for d in multi:
> for key, value in d.items():
> if key.lower() not in d.__class__._multivalued_fields:
> self.assert_(isinstance(value, unicode))
>
> def test_encoding_integrity(self):
> - utf8 = list(deb822.Deb822.iter_paragraphs(file('test_Sources')))
> + utf8 = list(deb822.Deb822.iter_paragraphs(file(data_path('test_Sources'))))
> latin1 = list(deb822.Deb822.iter_paragraphs(
> - file('test_Sources.iso8859-1'),
> + file(data_path('test_Sources.iso8859-1')),
> encoding='iso8859-1'))
>
> # dump() with no fd returns a unicode object - both should be identical
> @@ -686,9 +695,9 @@
> # XXX: The way multiline fields parsing works, we can't guarantee
> # that trailing whitespace is reproduced.
> utf8_contents = "\n".join([line.rstrip() for line in
> - file('test_Sources')] + [''])
> + file(data_path('test_Sources'))] + [''])
> latin1_contents = "\n".join([line.rstrip() for line in
> - file('test_Sources.iso8859-1')] + [''])
> + file(data_path('test_Sources.iso8859-1'))] + [''])
>
> utf8_to_latin1 = StringIO()
> for d in utf8:
> @@ -715,7 +724,7 @@
> # Avoid spitting out the encoding warning during testing.
> warnings.filterwarnings(action='ignore', category=UnicodeWarning)
>
> - filename = 'test_Sources.mixed_encoding'
> + filename = data_path('test_Sources.mixed_encoding')
> for paragraphs in [deb822.Sources.iter_paragraphs(file(filename)),
> deb822.Sources.iter_paragraphs(file(filename),
> use_apt_pkg=False)]:
> @@ -729,7 +738,7 @@
> class TestPkgRelations(unittest.TestCase):
>
> def test_packages(self):
> - pkgs = deb822.Packages.iter_paragraphs(file('test_Packages'))
> + pkgs = deb822.Packages.iter_paragraphs(file(data_path('test_Packages')))
> pkg1 = pkgs.next()
> rel1 = {'breaks': [],
> 'conflicts': [],
> @@ -804,7 +813,7 @@
> src_rel)))
>
> def test_sources(self):
> - pkgs = deb822.Sources.iter_paragraphs(file('test_Sources'))
> + pkgs = deb822.Sources.iter_paragraphs(file(data_path('test_Sources')))
> pkg1 = pkgs.next()
> rel1 = {'build-conflicts': [],
> 'build-conflicts-indep': [],
>
> === modified file 'tests/test_debfile.py'
> --- tests/test_debfile.py 2010-07-25 07:31:01 +0000
> +++ tests/test_debfile.py 2010-09-17 15:47:53 +0000
> @@ -19,21 +19,23 @@
>
> import unittest
> import os
> -import re
> import stat
> import sys
> -import tempfile
> import uu
>
> -sys.path.insert(0, '../lib/debian/')
> +sys.path.insert(0, '../lib')
>
> -import arfile
> -import debfile
> +from debian import arfile, debfile
I like these on different lines, but PEP 8 says it's ok. So I won't
complain either way. :)
>
> class TestArFile(unittest.TestCase):
>
> + def member_path(self, filename):
> + return os.path.join(os.path.dirname(__file__), filename)
See above (I'm also curious why you called this member_path instead of
data_path, as in test_deb822.py).
> +
> def setUp(self):
> - os.system("ar r test.ar test_debfile.py test_changelog test_deb822.py >/dev/null 2>&1")
> + data_path = os.path.dirname(__file__)
Maybe our hypothetical testing module data_path function or method
should have an optional filename argument, such that data_path() returns
the directory.
> + os.system("ar r test.ar " + " ".join([
> + os.path.join(data_path, filename) for filename in ["test_debfile.py", "test_changelog", "test_deb822.py"]]) + " >/dev/null 2>&1")
Or maybe it doesn't matter - here you could have used
os.system("ar r test.ar " + " ".join(map(member_path,
["test_debfile.py", "test_changelog", "test_deb822.py"]))
+ " >/dev/null 2>&1")
and dispensed with the data_path variable. 80 chars, too.
> assert os.path.exists("test.ar")
> self.testmembers = [ x.strip()
> for x in os.popen("ar t test.ar").readlines() ]
> @@ -42,7 +44,7 @@
> def tearDown(self):
> if os.path.exists('test.ar'):
> os.unlink('test.ar')
> -
> +
> def test_getnames(self):
> """ test for file list equality """
> self.assertEqual(self.a.getnames(), self.testmembers)
> @@ -53,8 +55,8 @@
> m = self.a.getmember(member)
> assert m
> self.assertEqual(m.name, member)
> -
> - mstat = os.stat(member)
> +
> + mstat = os.stat(self.member_path(member))
>
> self.assertEqual(m.size, mstat[stat.ST_SIZE])
> self.assertEqual(m.owner, mstat[stat.ST_UID])
> @@ -79,11 +81,11 @@
> def test_file_read(self):
> """ test for faked read """
> for m in self.a.getmembers():
> - f = open(m.name)
> -
> + f = open(self.member_path(m.name))
> +
> for i in [10, 100, 10000]:
> self.assertEqual(m.read(i), f.read(i))
> -
> +
> m.close()
> f.close()
>
> @@ -91,10 +93,10 @@
> """ test for faked readlines """
>
> for m in self.a.getmembers():
> - f = open(m.name)
> -
> + f = open(self.member_path(m.name))
> +
> self.assertEqual(m.readlines(), f.readlines())
> -
> +
> m.close()
> f.close()
>
> @@ -111,12 +113,13 @@
> self.debname = 'test.deb'
> self.broken_debname = 'test-broken.deb'
> self.bz2_debname = 'test-bz2.deb'
> - uudecode('test.deb.uu', self.debname)
> - uudecode('test-broken.deb.uu', self.broken_debname)
> - uudecode('test-bz2.deb.uu', self.bz2_debname)
> + data_path = os.path.dirname(__file__)
> + uudecode(os.path.join(data_path, 'test.deb.uu'), self.debname)
> + uudecode(os.path.join(data_path, 'test-broken.deb.uu'), self.broken_debname)
> + uudecode(os.path.join(data_path, 'test-bz2.deb.uu'), self.bz2_debname)
Likewise, wouldn't member_path('test.deb.uu') work?
>
> self.debname = 'test.deb'
> - uu_deb = open('test.deb.uu', 'r')
> + uu_deb = open(os.path.join(data_path, 'test.deb.uu'), 'r')
Ditto.
> bin_deb = open(self.debname, 'w')
> uu.decode(uu_deb, bin_deb)
> uu_deb.close()
>
> === modified file 'tests/test_debian_support.py'
> --- tests/test_debian_support.py 2010-03-14 11:37:42 +0000
> +++ tests/test_debian_support.py 2010-09-17 15:47:53 +0000
> @@ -21,10 +21,10 @@
> import sys
> import unittest
>
> -sys.path.insert(0, '../lib/debian/')
> +sys.path.insert(0, '../lib')
>
> -import debian_support
> -from debian_support import *
> +from debian import debian_support
> +from debian.debian_support import *
>
>
> class VersionTests(unittest.TestCase):
>
> === modified file 'tests/test_debtags.py'
> --- tests/test_debtags.py 2010-03-14 09:16:30 +0000
> +++ tests/test_debtags.py 2010-09-17 15:47:53 +0000
> @@ -17,16 +17,18 @@
> # along with this program; if not, write to the Free Software
> # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
>
> +import os
> import sys
> import unittest
>
> -sys.path.insert(0, '../lib/debian/')
> -import debtags
> +sys.path.insert(0, '../lib/')
> +from debian import debtags
>
> class TestDebtags(unittest.TestCase):
> +
> def mkdb(self):
> db = debtags.DB()
> - db.read(open("test_tagdb", "r"))
> + db.read(open(os.path.join(os.path.dirname(__file__), "test_tagdb"), "r"))
If you decide to make the testing-module data_path function/method,
please use it here too.
> return db
>
> def test_insert(self):
Second patch:
> === modified file 'debian/rules'
> --- debian/rules 2010-03-14 10:47:14 +0000
> +++ debian/rules 2010-09-20 17:22:45 +0000
> @@ -17,11 +17,7 @@
> python setup.py build
>
> # run the tests
> - cd tests && ./test_deb822.py
> - cd tests && ./test_debfile.py
> - cd tests && ./test_debtags.py
> - cd tests && ./test_changelog.py
> - cd tests && ./test_debian_support.py
> + ./tests/__init__.py
>
> lib/debian/doc-debtags > README.debtags
>
>
> === modified file 'tests/__init__.py' (properties changed: -x to +x)
> --- tests/__init__.py 2010-09-17 15:47:53 +0000
> +++ tests/__init__.py 2010-09-20 17:22:18 +0000
> @@ -33,3 +33,6 @@
> suite = loader.loadTestsFromNames(module_names)
> result.addTests(suite)
> return result
> +
> +if __name__ == '__main__':
> + unittest.main(defaultTest='test_suite')
I don't really like the idea of running an __init__.py... Can we make
an all_tests.py, something like:
import tests
if __name__ == '__main__':
unittest.main(defaultTest=tests.test_suite())
and run that file, instead of the tests package's __init__.py? (I
haven't actually tried this, but I assume it or something similar would
work.)
Again, thanks! Sorry for all the little nits.
--
John Wright <jsw at debian.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-python-debian-maint/attachments/20100922/5370cbe2/attachment-0001.pgp>
More information about the pkg-python-debian-maint
mailing list