No subject
Wed Jan 21 23:15:58 GMT 2009
them.
Ian> finally:
Ian> b.unlock()
Ian> @@ -1964,7 +2007,7 @@
Ian> """Take the input of a revision option and turn it into a revision range.
Ian> It returns RevisionInfo objects which can be used to obtain the rev_id's
Ian> - of the desired revisons. It does some user input validations.
Ian> + of the desired revisions. It does some user input validations.
Ian> """
Ian> if revisionspec_list is None:
Ian> rev1 = None
Ian> === modified file 'bzrlib/log.py'
Ian> --- bzrlib/log.py 2009-02-02 08:28:54 +0000
Ian> +++ bzrlib/log.py 2009-02-09 03:31:17 +0000
Ian> @@ -65,6 +65,7 @@
Ian> lazy_import(globals(), """
Ian> from bzrlib import (
Ian> + bzrdir,
Ian> config,
Ian> diff,
Ian> errors,
Ian> @@ -139,6 +140,99 @@
Ian> return rh
Ian> +class LogRequest(object):
Ian> + """Query parameters for logging a branch."""
Ian> +
Ian> + def __init__(self,
Ian> + direction='reverse',
Ian> + specific_fileids=None,
Ian> + start_revision=None,
Ian> + end_revision=None,
Ian> + limit=None,
Ian> + message_search=None,
Ian> + levels=1,
Ian> + generate_tags=True,
Ian> + delta_type=None,
Ian> + diff_type=None,
Ian> + _match_using_deltas=True,
Ian> + ):
Right. Not a single parameter is mandatory, you can as well pass
them to the log formatter constructor.
In many places below you make your additional classes manipulate
each other, so I don't think the separation is *that*
natural. Better leave them inside a single class until things get
clearer (if ever).
Ian> + """Create a logging request.
Ian> +
Ian> + Each of these parameter become a public attribute of the object.
Ian> +
Ian> + :param direction: 'reverse' (default) is latest to earliest;
Ian> + 'forward' is earliest to latest.
Please, can we just get rid of 'reverse' when it's not
*needed*. Or is there some strong reason to not use 'backward'
here ?
The difference between 'reverse' and 'backward', for me, is that
'backward' in itself has a meaning. 'reverse' by contrast only
makes sense if I know that it's used *against* 'forward'. That
knowledge is not available in all contexts.
Ian> +
Ian> + :param specific_fileids: If not None, only include revisions
Ian> + affecting the specified files, rather than all revisions.
Ian> +
Ian> + :param start_revision: If not None, only generate
Ian> + revisions >= start_revision
Ian> +
Ian> + :param end_revision: If not None, only generate
Ian> + revisions <= end_revision
Ian> +
I think there is a comment somewhere in log.py or a related
module that says that the API should really use revids. The
translation from revision specs to revids would be better handled
at the commands.Command level I think. At least, revid based API
will be easier to reuse between pull/push/missing which are
obvious users of this object.
Ian> +
Ian> + :param limit: If set, generate only 'limit'
Ian> +revisions, all revisions are shown if None or 0.
Ian> +
Ian> + :param message_search: If not None, only include revisions with
Ian> + matching commit messages
Ian> +
Ian> + :param levels: the number of levels of revisions to
Ian> + generate; 1 for just the mainline; 0 for all levels.
Ian> +
Ian> + :param generate_tags: If True, include tags for matched revisions.
Ian> +
Ian> + :param delta_type: Either 'full', 'partial' or None.
Ian> + 'full' means generate the complete delta - adds/deletes/modifies/etc;
Ian> + 'partial' means filter the delta using specific_fileids;
Ian> + None means do not generate any delta.
Ian> +
Ian> + :param diff_type: Either 'full', 'partial' or None.
Ian> + 'full' means generate the complete diff - adds/deletes/modifies/etc;
Ian> + 'partial' means filter the diff using specific_fileids;
Ian> + None means do not generate any diff.
Ian> +
Ian> + :param _match_using_deltas: a private parameter controlling the
Ian> + algorithm used for matching specific_fileids. This parameter
Ian> + may be removed in the future so bzrlib client code should NOT
Ian> + use it.
Ian> + """
Ian> + self.direction = direction
Ian> + self.specific_fileids = specific_fileids
Ian> + self.start_revision = start_revision
Ian> + self.end_revision = end_revision
Ian> + self.limit = limit
Ian> + self.message_search = message_search
Ian> + self.levels = levels
Ian> + self.generate_tags = generate_tags
Ian> + self.delta_type = delta_type
Ian> + self.diff_type = diff_type
Ian> + # Add 'private' attributes for features that may be deprecated
Ian> + self._match_using_deltas = _match_using_deltas
Ian> + self._allow_single_merge_revision = True
Ian> +
Ian> +
Ian> +def show_log_request(branch, lf, rqst):
Ian> + """Write out human-readable log of commits to this branch.
Ian> +
Ian> + :param lf: The LogFormatter object showing the output.
Ian> +
Ian> + :param rqst: The LogRequest object specifying the query parameters.
Ian> + """
Hmm. As I already said, I'd prefer a show_log(branch, revision, lf) API.
Now, revision (as in the revision Option object) can be a
parameter to lf constructor (for reasons not repeated here) and
we end up with show_log(branch, lf) which may even be simplified
to lf.show_log(branch).
Ian> + branch.lock_read()
Ian> + try:
Ian> + if getattr(lf, 'begin_log', None):
Ian> + lf.begin_log()
Ian> +
Ian> + _show_log_request(branch, lf, rqst)
Ian> +
Ian> + if getattr(lf, 'end_log', None):
Ian> + lf.end_log()
Ian> + finally:
Ian> + branch.unlock()
Ian> +
Ian> +
And by the arguments above, you have just duplicated the actual
show_log except the old parameters are now passed to log request
constructor.
So again, you just made it impossible for log formatters to
override many parts of the log formatting.
<snip/>
Ian> + # Tweak the LogRequest based on what the LogFormatter can handle.
Ian> + # (There's no point generating stuff if the formatter can't display it.)
Then just leave the log formatter decide that by itself.
<snip/>
Ian> +class _LogGenerator(object):
Ian> + """A generator of log revisions given a branch and a LogRequest."""
Same player shoot again :)
Again, how are we supposed to override this class ?
Merge it into log formatter and I'd be an happy camper.
And not only me, but any log formatter writer (and yourself when
it come to write the tests you forgot here :-)
My argument is that it should be possible to override any part of
log from a custom log formatter, that's what the log formatters
are for.
If you keep that design with three different classes, not only
will you need to provide ways to override them by a dedicated
registry, but you will also need to provide ways to specify them
from the command line and in the configuration files.
<snip/>
Ian> def _calc_view_revisions(branch, start_rev_id, end_rev_id, direction,
Ian> @@ -335,37 +449,54 @@
Ian> generate_single_revision = (end_rev_id and start_rev_id == end_rev_id and
Ian> (not generate_merge_revisions or not _has_merges(branch, end_rev_id)))
Ian> if generate_single_revision:
Ian> - if end_rev_id == br_rev_id:
Ian> - # It's the tip
Ian> - return [(br_rev_id, br_revno, 0)]
Ian> - else:
Ian> - revno = branch.revision_id_to_dotted_revno(end_rev_id)
Ian> - if len(revno) > 1 and not allow_single_merge_revision:
Ian> - # It's a merge revision and the log formatter is
Ian> - # completely brain dead. This "feature" of allowing
Ian> - # log formatters incapable of displaying dotted revnos
Ian> - # ought to be deprecated IMNSHO. IGC 20091022
Ian> - raise errors.BzrCommandError('Selected log formatter only'
Ian> - ' supports mainline revisions.')
Ian> - revno_str = '.'.join(str(n) for n in revno)
Ian> - return [(end_rev_id, revno_str, 0)]
Ian> + return _generate_one_revision(branch, end_rev_id, br_rev_id, br_revno,
Ian> + allow_single_merge_revision)
Ian> # If we only want to see linear revisions, we can iterate ...
Ian> if not generate_merge_revisions:
Ian> - result = _linear_view_revisions(branch, start_rev_id, end_rev_id)
Ian> - # If a start limit was given and it's not obviously an
Ian> - # ancestor of the end limit, check it before outputting anything
Ian> - if direction == 'forward' or (start_rev_id
Ian> - and not _is_obvious_ancestor(branch, start_rev_id, end_rev_id)):
Ian> - try:
Ian> - result = list(result)
Ian> - except _StartNotLinearAncestor:
Ian> - raise errors.BzrCommandError('Start revision not found in'
Ian> - ' left-hand history of end revision.')
Ian> - if direction == 'forward':
Ian> - result = reversed(list(result))
Ian> - return result
Ian> -
Ian> + return _generate_flat_revisions(branch, start_rev_id, end_rev_id,
Ian> + direction)
Ian> + else:
Ian> + return _generate_all_revisions(branch, start_rev_id, end_rev_id,
Ian> + direction, delayed_graph_generation)
Ian> +
Ian> +
Ian> +def _generate_one_revision(branch, rev_id, br_rev_id, br_revno,
Ian> + allow_single_merge_revision):
Ian> + if rev_id == br_rev_id:
Ian> + # It's the tip
Ian> + return [(br_rev_id, br_revno, 0)]
Ian> + else:
Ian> + revno = branch.revision_id_to_dotted_revno(rev_id)
Ian> + if len(revno) > 1 and not allow_single_merge_revision:
Ian> + # It's a merge revision and the log formatter is
Ian> + # completely brain dead. This "feature" of allowing
Ian> + # log formatters incapable of displaying dotted revnos
Ian> + # ought to be deprecated IMNSHO. IGC 20091022
Ian> + raise errors.BzrCommandError('Selected log formatter only'
Ian> + ' supports mainline revisions.')
Yeah, braindead, deprecate ! :)
Ian> + revno_str = '.'.join(str(n) for n in revno)
Ian> + return [(rev_id, revno_str, 0)]
Ian> +
Ian> +
Ian> +def _generate_flat_revisions(branch, start_rev_id, end_rev_id, direction):
Ian> + result = _linear_view_revisions(branch, start_rev_id, end_rev_id)
Ian> + # If a start limit was given and it's not obviously an
Ian> + # ancestor of the end limit, check it before outputting anything
Ian> + if direction == 'forward' or (start_rev_id
Ian> + and not _is_obvious_ancestor(branch, start_rev_id, end_rev_id)):
Ian> + try:
Ian> + result = list(result)
Ian> + except _StartNotLinearAncestor:
Ian> + raise errors.BzrCommandError('Start revision not found in'
Ian> + ' left-hand history of end revision.')
Ian> + if direction == 'forward':
Ian> + result = reversed(result)
This is the code I hate in conjunction with 'reverse' instead of
'backward', just for the sake of the example, the two lines above
can be written as:
if direction != 'reverse':
result = reversed(result)
Honestly, wouldn't you miss a heart beat if you were reading that ?
Ian> + return result
Ian> +
Ian> +
Ian> +def _generate_all_revisions(branch, start_rev_id, end_rev_id, direction,
Ian> + delayed_graph_generation):
I'm a bit surprised that this function and the preceding
(_generate_flat_revisions) are not in LogGenerator... do you have
some further refactoring plans ?
<snip/>
Ian> -def _generate_deltas(repository, log_rev_iterator, always_delta, fileids,
Ian> +def _generate_deltas(repository, log_rev_iterator, delta_type, fileids,
Ian> direction):
Ian> """Create deltas for each batch of revisions in log_rev_iterator.
Ian> @@ -646,50 +779,50 @@
Ian> if check_fileids and not fileid_set:
Ian> return
Ian> revisions = [rev[1] for rev in revs]
Ian> - deltas = repository.get_deltas_for_revisions(revisions)
Ian> new_revs = []
Ian> - for rev, delta in izip(revs, deltas):
Ian> - if check_fileids:
Ian> - if not _delta_matches_fileids(delta, fileid_set, stop_on):
Ian> - continue
Ian> - elif not always_delta:
Ian> - # Delta was created just for matching - ditch it
Ian> - # Note: It would probably be a better UI to return
Ian> - # a delta filtered by the file-ids, rather than
Ian> - # None at all. That functional enhancement can
Ian> - # come later ...
Ian> - delta = None
Ian> - new_revs.append((rev[0], rev[1], delta))
Ian> + if delta_type == 'full' and not check_fileids:
Ian> + deltas = repository.get_deltas_for_revisions(revisions)
Ian> + for rev, delta in izip(revs, deltas):
Ian> + new_revs.append((rev[0], rev[1], delta))
Ian> + else:
Ian> + deltas = repository.get_deltas_for_revisions(revisions, fileid_set)
Ian> + for rev, delta in izip(revs, deltas):
Ian> + if check_fileids:
Ian> + if delta is None or not delta.has_changed():
Ian> + continue
Ian> + else:
Ian> + _update_fileids(delta, fileid_set, stop_on)
Ian> + if delta_type is None:
Ian> + delta = None
Ian> + elif delta_type == 'full':
Ian> + # If the file matches all the time, rebuilding
Ian> + # a full delta like this in addition to a partial
Ian> + # one could be slow. However, it's likely the
s/the/that/ ?
Ian> +
Ian> + # most revisions won't get
Ian> + this far, making it # faster to filter on the
Ian> + partial deltas and # build the occasional full
Ian> + delta than always # building full deltas and
Ian> + filtering those. rev_id = rev[0][0] delta =
Ian> + repository.get_revision_delta(rev_id)
Ian> + new_revs.append((rev[0], rev[1], delta))
Ian> yield new_revs
Ian> -def _delta_matches_fileids(delta, fileids, stop_on='add'):
Ian> - """Check is a delta matches one of more file-ids.
Ian> +def _update_fileids(delta, fileids, stop_on):
This doesn't seem to be used elsewhere, nor tested independently,
so I'd find '_generate_deltas' more readable if you inline it
there.
Ian> def _make_revision_objects(branch, generate_delta, search, log_rev_iterator):
Ian> @@ -1570,55 +1703,97 @@
Ian> lf.log_revision(lr)
Ian> -def _get_fileid_to_log(revision, tree, b, fp):
Ian> - """Find the file-id to log for a file path in a revision range.
Ian> -
Ian> - :param revision: the revision range as parsed on the command line
Ian> - :param tree: the working tree, if any
Ian> - :param b: the branch
Ian> - :param fp: file path
Ian> +def _get_info_for_log_files(revisionspec_list, file_list):
Ian> + """Find file-ids and kinds given a list of files and a revision range.
Ian> +
Ian> + We search for files at the end of the range. If not found there,
Ian> + we try the start of the range.
Ian> +
Ian> + :param revisionspec_list: revision range as parsed on the command line
Ian> + :param file_list: the list of paths given on the command line;
Ian> + the first of these can be a branch location or a file path,
Ian> + the remainder must be file paths
Ian> + :return: (branch, info_list, start_rev_info, end_rev_info) where
Ian> + info_list is a list of (relative_path, file_id, kind) tuples where
Ian> + kind is one of values 'directory', 'file', 'symlink', 'tree-reference'.
Ian> """
Ian> - if revision is None:
Ian> + from builtins import _get_revision_range, safe_relpath_files
Ian> + tree, b, path = bzrdir.BzrDir.open_containing_tree_or_branch(file_list[0])
Ian> + # XXX: It's damn messy converting a list of paths to relative paths when
Ian> + # those paths might be deleted ones, they might be on a case-insensitive
Ian> + # filesystem and/or they might be in silly locations (like another branch).
Ian> + # For example, what should "log bzr://branch/dir/file1 file2" do? (Is
Ian> + # file2 implicitly in the same dir as file1 or should its directory be
Ian> + # taken from the current tree somehow?) For now, this solves the common
Ian> + # case of running log in a nested directory, assuming paths beyond the
Ian> + # first one haven't been deleted ...
Your comment already defined many tests :-)
You don't have to write the ones that you expect to fail with
your actual implementation, but this has proven very useful in at
least one occasion (lca merges), so I would warmly recommand it
anyway :)
...
Very warmly.
Is 'hotly' english ? :)
<snip/>
Ian> === modified file 'bzrlib/tests/blackbox/test_log.py'
You add only blackbox tests ?
Given the modifications above I was expecting a lot of tests for
the helpers and the new classes added....
Ian> --- bzrlib/tests/blackbox/test_log.py 2009-01-31 04:17:43 +0000
Ian> +++ bzrlib/tests/blackbox/test_log.py 2009-02-09 03:31:17 +0000
Ian> @@ -243,14 +243,27 @@
Ian> self.assertContainsRe(log, r'revno: 2\n')
Ian> self.assertContainsRe(log, r'revno: 3\n')
Ian> + def test_log_all_files_usage(self):
Ian> + self._prepare()
Ian> + err = self.run_bzr('log --all-files', retcode=3)[1]
Ian> + self.assertContainsRe(err, '--all-files is only applicable'
Ian> + ' when files and -v or -p are specified')
Ian> + err = self.run_bzr('log --all-files hello.txt', retcode=3)[1]
Ian> + self.assertContainsRe(err, '--all-files is only applicable'
Ian> + ' when files and -v or -p are specified')
Ian> + err = self.run_bzr('log --all-files -v hello.txt')[1]
Ian> + self.assertContainsRe(err, '')
Ian> + err = self.run_bzr('log --all-files -p hello.txt')[1]
Ian> + self.assertContainsRe(err, '')
Ian> +
Hmm, what the word ? Defect localization ? Yeah, can you enhance
defect localization here ?
At least separating the retcode=3 (which may not even need the
self_prepare()) from the others ?
Ian> class TestLogVerbose(TestCaseWithTransport):
Ian> def setUp(self):
Ian> super(TestLogVerbose, self).setUp()
Ian> tree = self.make_branch_and_tree('.')
Ian> - self.build_tree(['hello.txt'])
Ian> - tree.add('hello.txt')
Ian> + self.build_tree(['hello.txt', 'world.txt'])
Ian> + tree.add(['hello.txt', 'world.txt'])
Ian> tree.commit(message='message1')
Ian> def assertUseShortDeltaFormat(self, cmd):
Ian> @@ -281,6 +294,41 @@
Ian> # level
Ian> self.assertUseLongDeltaFormat(['log', '--long', '-vv'])
Ian> + def test_log_long_verbose_file(self):
Ian> + # When filtering by paths, filter the delta as well by default
Ian> + out, err = self.run_bzr('log --long -v hello.txt')
Ian> + self.assertEqual('', err)
Ian> + log = normalize_log(out)
Ian> + self.assertEqualDiff(log, """\
Ian> +------------------------------------------------------------
Ian> +revno: 1
Ian> +committer: Lorem Ipsum <test at example.com>
Ian> +branch nick: work
Ian> +timestamp: Just now
Ian> +message:
Ian> + message1
Ian> +added:
Ian> + hello.txt
Ian> +""")
Ian> +
Ian> + def test_log_long_verbose_file_all_files(self):
Ian> + # When filtering by paths, --all-files shows the full delta
Ian> + out, err = self.run_bzr('log --long -v --all-files hello.txt')
Ian> + self.assertEqual('', err)
Ian> + log = normalize_log(out)
Ian> + self.assertEqualDiff(log, """\
Ian> +------------------------------------------------------------
Ian> +revno: 1
Ian> +committer: Lorem Ipsum <test at example.com>
Ian> +branch nick: work
Ian> +timestamp: Just now
Ian> +message:
Ian> + message1
Ian> +added:
Ian> + hello.txt
Ian> + world.txt
Ian> +""")
Ian> +
Both of these tests will be easier to read and maintain if we
have a test log formatter that will just record the revisions it
should display. The tests could then just make assertions on the
recorded revision list without going into formatting details we
really don't care about here.
Ian> class TestLogMerges(TestCaseWithoutPropsHandler):
Ian> @@ -502,6 +550,12 @@
Ian> 'YYYY-MM-DD HH:MM:SS +ZZZZ', string)
Ian> +def subst_short_dates(string):
Ian> + """Replace date strings (without time or timezone) with constant values."""
Ian> + return re.sub(r'\d{4}-\d{2}-\d{2}',
Ian> + 'YYYY-MM-DD', string)
Ian> +
Ian> +
Ian> class TestLogDiff(TestCaseWithoutPropsHandler):
Ian> def _prepare(self):
Ian> @@ -659,6 +713,37 @@
Ian> """)
Ian> + def test_log_show_diff_file_all_files(self):
Ian> + self._prepare()
Ian> + out,err = self.run_bzr('log -p --short --all-files file2')
Ian> + self.assertEqual('', err)
Ian> + log = normalize_log(out)
Ian> + self.assertEqualDiff(subst_dates(log), """\
Ian> + 2 Lorem Ipsum\t2005-11-22 [merge]
Ian> + merge branch 1
Ian> + === modified file 'file2'
Ian> + --- file2\tYYYY-MM-DD HH:MM:SS +ZZZZ
Ian> + +++ file2\tYYYY-MM-DD HH:MM:SS +ZZZZ
Ian> + @@ -1,1 +1,1 @@
Ian> + -contents of parent/file2
Ian> + +hello
Ian> +
Ian> + 1 Lorem Ipsum\t2005-11-22
Ian> + first post
Ian> + === added file 'file1'
Ian> + --- file1\tYYYY-MM-DD HH:MM:SS +ZZZZ
Ian> + +++ file1\tYYYY-MM-DD HH:MM:SS +ZZZZ
Ian> + @@ -0,0 +1,1 @@
Ian> + +contents of parent/file1
Ian> +
Ian> + === added file 'file2'
Ian> + --- file2\tYYYY-MM-DD HH:MM:SS +ZZZZ
Ian> + +++ file2\tYYYY-MM-DD HH:MM:SS +ZZZZ
Ian> + @@ -0,0 +1,1 @@
Ian> + +contents of parent/file2
Ian> +
Ian> +""")
Ian> +
Same here, or may be a special purpose log formatter where you
redefine the method that handle the diff display... and *only*
that one.
Ian> class TestLogEncodings(TestCaseInTempDir):
Ian> @@ -954,3 +1039,105 @@
Ian> self.assertNotContainsRe(log, '^3:', re.MULTILINE)
Ian> self.assertNotContainsRe(log, '^3.1.1:', re.MULTILINE)
Ian> self.assertNotContainsRe(log, '^4:', re.MULTILINE)
Ian> +
Ian> +
Ian> +class TestLogMultiple(TestCaseWithTransport):
Ian> +
Ian> + def prepare_tree(self):
Ian> + tree = self.make_branch_and_tree('parent')
Ian> + self.build_tree([
Ian> + 'parent/file1',
Ian> + 'parent/file2',
Ian> + 'parent/dir1/',
Ian> + 'parent/dir1/file5',
Ian> + 'parent/dir1/dir2/',
Ian> + 'parent/dir1/dir2/file3',
Ian> + 'parent/file4'])
Ian> + tree.add('file1')
Ian> + tree.commit('add file1', committer='Lorem Ipsum <test at example.com>')
Ian> + tree.add('file2')
Ian> + tree.commit('add file2', committer='Lorem Ipsum <test at example.com>')
Ian> + tree.add(['dir1', 'dir1/dir2', 'dir1/dir2/file3'])
Ian> + tree.commit('add file3', committer='Lorem Ipsum <test at example.com>')
Ian> + tree.add('file4')
Ian> + tree.commit('add file4', committer='Lorem Ipsum <test at example.com>')
Ian> + tree.add('dir1/file5')
Ian> + tree.commit('add file5', committer='Lorem Ipsum <test at example.com>')
You add the commiter to make it easier to match the results later.
Again a test log formatter will make that useless and make the
test easier to read.
Ian> + child_tree = tree.bzrdir.sprout('child').open_workingtree()
Ian> + self.build_tree_contents([('child/file2', 'hello')])
Ian> + child_tree.commit(message='branch 1',
Ian> + committer='Lorem Ipsum <test at example.com>')
Ian> + tree.merge_from_branch(child_tree.branch)
Ian> + tree.commit(message='merge child branch',
Ian> + committer='Lorem Ipsum <test at example.com>')
You even have to split the lines here...
Ian> + os.chdir('parent')
Ian> +
Ian> + def test_log_files(self):
Ian> + """The log for multiple file should only list revs for those files"""
Ian> + self.prepare_tree()
Ian> + out, err = self.run_bzr('log --line -n0 file1 file2 dir1/dir2/file3')
Ian> + self.assertEqual('', err)
Ian> + log = normalize_log(out)
Ian> + self.assertEqualDiff(subst_short_dates(log), """\
Ian> +6: Lorem Ipsum YYYY-MM-DD merge child branch
Ian> + 5.1.1: Lorem Ipsum YYYY-MM-DD branch 1
Ian> +3: Lorem Ipsum YYYY-MM-DD add file3
Ian> +2: Lorem Ipsum YYYY-MM-DD add file2
Ian> +1: Lorem Ipsum YYYY-MM-DD add file1
Ian> +""")
S/n ratio very bad, compare:
self.assertEquals(['6', '5.1.1', '3', '2', '1'], self.get_revnos(test_lf))
or test_lf_get_revnos(), that's not the point here.
Ian> +
Ian> + def test_log_directory(self):
Ian> + """The log for a directory should show all nested files."""
Ian> + self.prepare_tree()
Ian> + out, err = self.run_bzr('log --line -n0 dir1')
Ian> + self.assertEqual('', err)
Ian> + log = normalize_log(out)
Ian> + self.assertEqualDiff(subst_short_dates(log), """\
Ian> +5: Lorem Ipsum YYYY-MM-DD add file5
Ian> +3: Lorem Ipsum YYYY-MM-DD add file3
Ian> +""")
self.assertEquals(['5', '3'], self.get_revnos(test_lf))
Ian> +
Ian> + def test_log_nested_directory(self):
Ian> + """The log for a directory should show all nested files."""
Ian> + self.prepare_tree()
Ian> + out, err = self.run_bzr('log --line -n0 dir1/dir2')
Ian> + self.assertEqual('', err)
Ian> + log = normalize_log(out)
Ian> + self.assertEqualDiff(subst_short_dates(log), """\
Ian> +3: Lorem Ipsum YYYY-MM-DD add file3
self.assertEquals(['3'], self.get_revnos(test_lf))
etc...
Ian> +""")
Ian> +
Ian> + def test_log_in_nested_directory(self):
Ian> + """The log for a directory should show all nested files."""
Ian> + self.prepare_tree()
But anyway, the code/tests ratio seems far too high here,
especially in an area very sensible, UI-wise and performance
wise.
I expect bugs here will go undetected even with a careful review.
Vincent
More information about the bazaar
mailing list