Rev 4809: Review feedback. in http://bazaar.launchpad.net/~lifeless/bzr/subunit
Robert Collins
robertc at robertcollins.net
Wed Dec 16 22:29:52 GMT 2009
At http://bazaar.launchpad.net/~lifeless/bzr/subunit
------------------------------------------------------------
revno: 4809
revision-id: robertc at robertcollins.net-20091216222931-wbbn5ey4mwmpatwd
parent: robertc at robertcollins.net-20091215232824-yewvn7pn7baaevux
committer: Robert Collins <robertc at robertcollins.net>
branch nick: subunit
timestamp: Thu 2009-12-17 09:29:31 +1100
message:
Review feedback.
=== modified file 'NEWS'
--- a/NEWS 2009-12-15 23:28:24 +0000
+++ b/NEWS 2009-12-16 22:29:31 +0000
@@ -32,10 +32,10 @@
Internals
*********
-* New helper osutils.StreamWriter which encodes unicode objects but
- passes str objects straight through. This is used for selftest but
- may be useful for diff and other operations that generate mixed output.
- (Robert Collins)
+* New helper osutils.UnicodeOrBytesToBytesWriter which encodes unicode
+ objects but passes str objects straight through. This is used for
+ selftest but may be useful for diff and other operations that generate
+ mixed output. (Robert Collins)
Testing
*******
@@ -43,7 +43,9 @@
* ``bzrlib.tests.TestCase`` now subclasses ``testtools.testcase.TestCase``.
This permits features in testtools such as getUniqueInteger and
getUniqueString to be used. Because of this, testtools version 0.9.2 or
- newer is now a dependency to run bzr selftest. (Robert Collins)
+ newer is now a dependency to run bzr selftest. Running with versions of
+ testtools less than 0.9.2 will cause bzr to error while loading the test
+ suite. (Robert Collins)
bzr 2.0.4 (not released yet)
############################
=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py 2009-12-15 23:28:24 +0000
+++ b/bzrlib/osutils.py 2009-12-16 22:29:31 +0000
@@ -2074,7 +2074,7 @@
return concurrency
-class StreamWriter(codecs.StreamWriter):
+class UnicodeOrBytesToBytesWriter(codecs.StreamWriter):
"""A stream writer that doesn't decode str arguments."""
def __init__(self, codec, stream, errors='strict'):
@@ -2082,7 +2082,7 @@
self.encode = codec.encode
def write(self, object):
- if type(object) == str:
+ if type(object) is str:
self.stream.write(object)
else:
data, _ = self.encode(object, self.errors)
=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py 2009-12-15 23:28:24 +0000
+++ b/bzrlib/tests/__init__.py 2009-12-16 22:29:31 +0000
@@ -484,8 +484,7 @@
))
def report_known_failure(self, test, err):
- ui.ui_factory.note('XFAIL: %s\n%s\n' % (
- self._test_description(test), err[1]))
+ pass
def report_skip(self, test, reason):
pass
@@ -595,7 +594,7 @@
# to encode using ascii.
new_encoding = osutils.get_terminal_encoding()
codec = codecs.lookup(new_encoding)
- stream = osutils.StreamWriter(codec, stream)
+ stream = osutils.UnicodeOrBytesToBytesWriter(codec, stream)
stream.encoding = new_encoding
self.stream = unittest._WritelnDecorator(stream)
self.descriptions = descriptions
@@ -672,15 +671,8 @@
traceback._some_str = _clever_some_str
-
-class KnownFailure(AssertionError):
- """Indicates that a test failed in a precisely expected manner.
-
- Such failures dont block the whole test suite from passing because they are
- indicators of partially completed code or of future work. We have an
- explicit error for them so that we can ensure that they are always visible:
- KnownFailures are always shown in the output of bzr selftest.
- """
+# deprecated
+KnownFailure = testtools.testcase._ExpectedFailure
class UnavailableFeature(Exception):
@@ -798,8 +790,6 @@
(UnavailableFeature, self._do_unsupported_or_skip))
self.exception_handlers.insert(0,
(TestNotApplicable, self._do_not_applicable))
- self.exception_handlers.insert(0,
- (KnownFailure, self._do_known_failure))
def setUp(self):
super(TestCase, self).setUp()
@@ -1293,41 +1283,6 @@
m += ": " + msg
self.fail(m)
- def expectFailure(self, reason, assertion, *args, **kwargs):
- """Invoke a test, expecting it to fail for the given reason.
-
- This is for assertions that ought to succeed, but currently fail.
- (The failure is *expected* but not *wanted*.) Please be very precise
- about the failure you're expecting. If a new bug is introduced,
- AssertionError should be raised, not KnownFailure.
-
- Frequently, expectFailure should be followed by an opposite assertion.
- See example below.
-
- Intended to be used with a callable that raises AssertionError as the
- 'assertion' parameter. args and kwargs are passed to the 'assertion'.
-
- Raises KnownFailure if the test fails. Raises AssertionError if the
- test succeeds.
-
- example usage::
-
- self.expectFailure('Math is broken', self.assertNotEqual, 54,
- dynamic_val)
- self.assertEqual(42, dynamic_val)
-
- This means that a dynamic_val of 54 will cause the test to raise
- a KnownFailure. Once math is fixed and the expectFailure is removed,
- only a dynamic_val of 42 will allow the test to pass. Anything other
- than 54 or 42 will cause an AssertionError.
- """
- try:
- assertion(*args, **kwargs)
- except AssertionError:
- raise KnownFailure(reason)
- else:
- self.fail('Unexpected success. Should have failed: %s' % reason)
-
def assertFileEqual(self, content, path):
"""Fail if path does not contain 'content'."""
self.failUnlessExists(path)
@@ -1643,7 +1598,10 @@
mutter(*args)
def _get_log(self, keep_log_file=False):
- """Get the log from bzrlib.trace calls from this test.
+ """Internal helper to get the log from bzrlib.trace for this test.
+
+ Please use self.getDetails, or self.get_log to access this in test case
+ code.
:param keep_log_file: When True, if the log is still a file on disk
leave it as a file on disk. When False, if the log is still a file
@@ -1692,6 +1650,13 @@
else:
return "No log file content and no log file name."
+ def get_log(self):
+ """Get a unicode string containing the log from bzrlib.trace.
+
+ Undecodable characters are replaced.
+ """
+ return u"".join(self.getDetails()['log'].iter_text())
+
def requireFeature(self, feature):
"""This test requires a specific feature is available.
=== modified file 'bzrlib/tests/blackbox/test_debug.py'
--- a/bzrlib/tests/blackbox/test_debug.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/blackbox/test_debug.py 2009-12-16 22:29:31 +0000
@@ -38,5 +38,4 @@
def test_dash_dlock(self):
# With -Dlock, locking and unlocking is recorded into the log
self.run_bzr("-Dlock init foo")
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertContainsRe(log, "lock_write")
+ self.assertContainsRe(self.get_log(), "lock_write")
=== modified file 'bzrlib/tests/blackbox/test_exceptions.py'
--- a/bzrlib/tests/blackbox/test_exceptions.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/blackbox/test_exceptions.py 2009-12-16 22:29:31 +0000
@@ -55,8 +55,7 @@
os.mkdir('foo')
bzrdir.BzrDirFormat5().initialize('foo')
out, err = self.run_bzr("status foo")
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertContainsRe(log, "bzr upgrade")
+ self.assertContainsRe(self.get_log(), "bzr upgrade")
finally:
repository._deprecation_warning_done = True
=== modified file 'bzrlib/tests/per_pack_repository.py'
--- a/bzrlib/tests/per_pack_repository.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/per_pack_repository.py 2009-12-16 22:29:31 +0000
@@ -687,7 +687,7 @@
# abort_write_group will not raise an error
self.assertEqual(None, repo.abort_write_group(suppress_errors=True))
# But it does log an error
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
self.assertContainsRe(log, 'abort_write_group failed')
self.assertContainsRe(log, r'INFO bzr: ERROR \(ignored\):')
if token is not None:
=== modified file 'bzrlib/tests/per_repository/test_check.py'
--- a/bzrlib/tests/per_repository/test_check.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/per_repository/test_check.py 2009-12-16 22:29:31 +0000
@@ -46,8 +46,7 @@
revid2 = tree.commit('2')
check_object = tree.branch.repository.check([revid1, revid2])
check_object.report_results(verbose=True)
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertContainsRe(log, "0 unreferenced text versions")
+ self.assertContainsRe(self.get_log(), "0 unreferenced text versions")
class TestFindInconsistentRevisionParents(TestCaseWithBrokenRevisionIndex):
@@ -90,13 +89,11 @@
# contents of it!
check_object = repo.check(['ignored'])
check_object.report_results(verbose=False)
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertContainsRe(
- log, '1 revisions have incorrect parents in the revision index')
+ self.assertContainsRe(self.get_log(),
+ '1 revisions have incorrect parents in the revision index')
check_object.report_results(verbose=True)
- log = u"".join(self.getDetails()['log'].iter_text())
self.assertContainsRe(
- log,
+ self.get_log(),
"revision-id has wrong parents in index: "
r"\('incorrect-parent',\) should be \(\)")
@@ -147,5 +144,5 @@
rev_id = builder.commit('first post')
result = repo.check(None, check_repo=True)
result.report_results(True)
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
self.assertFalse('Missing' in log, "Something was missing in %r" % log)
=== modified file 'bzrlib/tests/per_repository/test_check_reconcile.py'
--- a/bzrlib/tests/per_repository/test_check_reconcile.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/per_repository/test_check_reconcile.py 2009-12-16 22:29:31 +0000
@@ -198,7 +198,7 @@
repo, scenario = self.prepare_test_repository()
check_result = repo.check()
check_result.report_results(verbose=True)
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
for pattern in scenario.check_regexes(repo):
self.assertContainsRe(log, pattern)
=== modified file 'bzrlib/tests/per_repository_reference/test_check.py'
--- a/bzrlib/tests/per_repository_reference/test_check.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/per_repository_reference/test_check.py 2009-12-16 22:29:31 +0000
@@ -39,5 +39,4 @@
check_result = referring.branch.repository.check(
referring.branch.repository.all_revision_ids())
check_result.report_results(verbose=False)
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertFalse("inconsistent parents" in log)
+ self.assertFalse("inconsistent parents" in self.get_log())
=== modified file 'bzrlib/tests/test_cleanup.py'
--- a/bzrlib/tests/test_cleanup.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/test_cleanup.py 2009-12-16 22:29:31 +0000
@@ -39,8 +39,7 @@
self.call_log.append('no_op_cleanup')
def assertLogContains(self, regex):
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertContainsRe(log, regex, re.DOTALL)
+ self.assertContainsRe(self.get_log(), regex, re.DOTALL)
def failing_cleanup(self):
self.call_log.append('failing_cleanup')
@@ -184,8 +183,7 @@
self.assertRaises(ErrorA, _do_with_cleanups, cleanups,
self.trivial_func)
self.assertLogContains('Cleanup failed:.*ErrorB')
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertFalse('ErrorA' in log)
+ self.assertFalse('ErrorA' in self.get_log())
def make_two_failing_cleanup_funcs(self):
def raise_a():
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2009-12-15 23:28:24 +0000
+++ b/bzrlib/tests/test_config.py 2009-12-16 22:29:31 +0000
@@ -1599,9 +1599,8 @@
# the user is prompted
self.assertEquals(entered_password,
conf.get_password('ssh', 'bar.org', user='jim'))
- log = u"".join(self.getDetails()['log'].iter_text())
self.assertContainsRe(
- log,
+ self.log_log(),
'password ignored in section \[ssh with password\]')
def test_ssh_without_password_doesnt_emit_warning(self):
@@ -1625,9 +1624,8 @@
conf.get_password('ssh', 'bar.org', user='jim'))
# No warning shoud be emitted since there is no password. We are only
# providing "user".
- log = u"".join(self.getDetails()['log'].iter_text())
self.assertNotContainsRe(
- log,
+ self.get_log(),
'password ignored in section \[ssh with password\]')
def test_uses_fallback_stores(self):
=== modified file 'bzrlib/tests/test_http_response.py'
--- a/bzrlib/tests/test_http_response.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/test_http_response.py 2009-12-16 22:29:31 +0000
@@ -96,8 +96,7 @@
# Override the thresold to force the warning emission
conn._range_warning_thresold = 6 # There are 7 bytes pending
conn.cleanup_pipe()
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertContainsRe(log, 'Got a 200 response when asking')
+ self.assertContainsRe(self.get_log(), 'Got a 200 response when asking')
class TestRangeFileMixin(object):
=== modified file 'bzrlib/tests/test_merge.py'
--- a/bzrlib/tests/test_merge.py 2009-12-15 23:28:24 +0000
+++ b/bzrlib/tests/test_merge.py 2009-12-16 22:29:31 +0000
@@ -152,13 +152,12 @@
log = StringIO()
merge_inner(tree_b.branch, tree_a, tree_b.basis_tree(),
this_tree=tree_b, ignore_zero=True)
- log = u"".join(self.getDetails()['log'].iter_text())
- self.failUnless('All changes applied successfully.\n' not in log)
+ self.failUnless('All changes applied successfully.\n' not in
+ self.get_log())
tree_b.revert()
merge_inner(tree_b.branch, tree_a, tree_b.basis_tree(),
this_tree=tree_b, ignore_zero=False)
- log = u"".join(self.getDetails()['log'].iter_text())
- self.failUnless('All changes applied successfully.\n' in log)
+ self.failUnless('All changes applied successfully.\n' in self.get_log())
def test_merge_inner_conflicts(self):
tree_a = self.make_branch_and_tree('a')
=== modified file 'bzrlib/tests/test_remote.py'
--- a/bzrlib/tests/test_remote.py 2009-12-15 23:28:24 +0000
+++ b/bzrlib/tests/test_remote.py 2009-12-16 22:29:31 +0000
@@ -2892,9 +2892,8 @@
self.assertEqual(server_error, translated_error)
# In addition to re-raising ErrorFromSmartServer, some debug info has
# been muttered to the log file for developer to look at.
- log = u"".join(self.getDetails()['log'].iter_text())
self.assertContainsRe(
- log,
+ self.get_log(),
"Missing key 'branch' in context")
def test_path_missing(self):
@@ -2908,8 +2907,7 @@
self.assertEqual(server_error, translated_error)
# In addition to re-raising ErrorFromSmartServer, some debug info has
# been muttered to the log file for developer to look at.
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertContainsRe(log, "Missing key 'path' in context")
+ self.assertContainsRe(self.get_log(), "Missing key 'path' in context")
class TestStacking(tests.TestCaseWithTransport):
=== modified file 'bzrlib/tests/test_selftest.py'
--- a/bzrlib/tests/test_selftest.py 2009-12-15 23:28:24 +0000
+++ b/bzrlib/tests/test_selftest.py 2009-12-16 22:29:31 +0000
@@ -97,7 +97,8 @@
log = details['log']
self.assertThat(log.content_type, Equals(ContentType(
"text", "plain", {"charset": "utf8"})))
- self.assertThat(u"".join(log.iter_text()),
+ self.assertThat(u"".join(log.iter_text()), Equals(self.get_log()))
+ self.assertThat(self.get_log(),
DocTestMatches(u"...a test message\n", ELLIPSIS))
@@ -847,8 +848,8 @@
def stopTestRun(self): pass
def startTests(self): pass
def report_test_start(self, test): pass
- def report_known_failure(self, test, err):
- self._call = test, err
+ def report_known_failure(self, test, err=None, details=None):
+ self._call = test, 'known failure'
result = InstrumentedTestResult(None, None, None, None)
class Test(tests.TestCase):
def test_function(self):
@@ -858,8 +859,7 @@
# it should invoke 'report_known_failure'.
self.assertEqual(2, len(result._call))
self.assertEqual(test.id(), result._call[0].id())
- self.assertEqual(tests.KnownFailure, result._call[1][0])
- self.assertIsInstance(result._call[1][1], tests.KnownFailure)
+ self.assertEqual('known failure', result._call[1])
# we dont introspec the traceback, if the rest is ok, it would be
# exceptional for it not to be.
# it should update the known_failure_count on the object.
@@ -1043,11 +1043,11 @@
# the final output when real failures occur.
class Test(tests.TestCase):
def known_failure_test(self):
- raise tests.KnownFailure('failed')
+ self.expectFailure('failed', self.assertTrue, False)
test = unittest.TestSuite()
test.addTest(Test("known_failure_test"))
def failing_test():
- raise AssertionError('foo')
+ self.fail('foo')
test.addTest(unittest.FunctionTestCase(failing_test))
stream = StringIO()
runner = tests.TextTestRunner(stream=stream)
@@ -1061,7 +1061,7 @@
'^----------------------------------------------------------------------\n'
'Traceback \\(most recent call last\\):\n'
' .*' # File .*, line .*, in failing_test' - but maybe not from .pyc
- ' raise AssertionError\\(\'foo\'\\)\n'
+ ' self.fail\\(\'foo\'\\)\n'
'.*'
'^----------------------------------------------------------------------\n'
'.*'
@@ -1073,7 +1073,7 @@
# the final output.
class Test(tests.TestCase):
def known_failure_test(self):
- raise tests.KnownFailure('failed')
+ self.expectFailure('failed', self.assertTrue, False)
test = Test("known_failure_test")
stream = StringIO()
runner = tests.TextTestRunner(stream=stream)
@@ -2340,28 +2340,6 @@
self.assertEqual('bzr: interrupted\n', result[1])
-class TestKnownFailure(tests.TestCase):
-
- def test_known_failure(self):
- """Check that KnownFailure is defined appropriately."""
- # a KnownFailure is an assertion error for compatability with unaware
- # runners.
- self.assertIsInstance(tests.KnownFailure(""), AssertionError)
-
- def test_expect_failure(self):
- try:
- self.expectFailure("Doomed to failure", self.assertTrue, False)
- except tests.KnownFailure, e:
- self.assertEqual('Doomed to failure', e.args[0])
- try:
- self.expectFailure("Doomed to failure", self.assertTrue, True)
- except AssertionError, e:
- self.assertEqual('Unexpected success. Should have failed:'
- ' Doomed to failure', e.args[0])
- else:
- self.fail('Assertion not raised')
-
-
class TestFeature(tests.TestCase):
def test_caching(self):
@@ -2603,8 +2581,7 @@
# the test framework
self.assertEquals('always fails', str(e))
# check that there's no traceback in the test log
- log = u"".join(self.getDetails()['log'].iter_text())
- self.assertNotContainsRe(log, r'Traceback')
+ self.assertNotContainsRe(self.get_log(), r'Traceback')
def test_run_bzr_user_error_caught(self):
# Running bzr in blackbox mode, normal/expected/user errors should be
@@ -2919,7 +2896,7 @@
tpr.register('bar', 'bbb.aaa.rrr')
tpr.register('bar', 'bBB.aAA.rRR')
self.assertEquals('bbb.aaa.rrr', tpr.get('bar'))
- self.assertThat(u"".join(self.getDetails()['log'].iter_text()),
+ self.assertThat(self.get_log(),
DocTestMatches("...bar...bbb.aaa.rrr...BB.aAA.rRR", ELLIPSIS))
def test_get_unknown_prefix(self):
=== modified file 'bzrlib/tests/test_trace.py'
--- a/bzrlib/tests/test_trace.py 2009-12-15 23:28:24 +0000
+++ b/bzrlib/tests/test_trace.py 2009-12-16 22:29:31 +0000
@@ -144,20 +144,20 @@
def test_trace_unicode(self):
"""Write Unicode to trace log"""
self.log(u'the unicode character for benzene is \N{BENZENE RING}')
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
self.assertContainsRe(log, "the unicode character for benzene is")
def test_trace_argument_unicode(self):
"""Write a Unicode argument to the trace log"""
mutter(u'the unicode character for benzene is %s', u'\N{BENZENE RING}')
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
self.assertContainsRe(log, 'the unicode character')
def test_trace_argument_utf8(self):
"""Write a Unicode argument to the trace log"""
mutter(u'the unicode character for benzene is %s',
u'\N{BENZENE RING}'.encode('utf-8'))
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
self.assertContainsRe(log, 'the unicode character')
def test_report_broken_pipe(self):
@@ -177,7 +177,7 @@
def test_mutter_callsite_1(self):
"""mutter_callsite can capture 1 level of stack frame."""
mutter_callsite(1, "foo %s", "a string")
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
# begin with the message
self.assertLogStartsWith(log, 'foo a string\nCalled from:\n')
# should show two frame: this frame and the one above
@@ -189,7 +189,7 @@
def test_mutter_callsite_2(self):
"""mutter_callsite can capture 2 levels of stack frame."""
mutter_callsite(2, "foo %s", "a string")
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
# begin with the message
self.assertLogStartsWith(log, 'foo a string\nCalled from:\n')
# should show two frame: this frame and the one above
@@ -206,7 +206,7 @@
mutter(u'Writing a greek mu (\xb5) works in a unicode string')
mutter('But fails in an ascii string \xb5')
mutter('and in an ascii argument: %s', '\xb5')
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
self.assertContainsRe(log, 'Writing a greek mu')
self.assertContainsRe(log, "But fails in an ascii string")
# However, the log content object does unicode replacement on reading
=== modified file 'bzrlib/tests/test_transport_log.py'
--- a/bzrlib/tests/test_transport_log.py 2009-12-06 00:12:45 +0000
+++ b/bzrlib/tests/test_transport_log.py 2009-12-16 22:29:31 +0000
@@ -36,7 +36,7 @@
# operations such as mkdir are logged
mutter('where are you?')
logging_transport.mkdir('subdir')
- log = u"".join(self.getDetails()['log'].iter_text())
+ log = self.get_log()
self.assertContainsRe(log, r'mkdir memory\+\d+://.*subdir')
self.assertContainsRe(log, ' --> None')
# they have the expected effect
More information about the bazaar-commits
mailing list