[MERGE] inode sorted stats in status
Andrew Bennetts
andrew at canonical.com
Tue Sep 2 03:32:28 BST 2008
bb:tweak
I don't share John's concerns about merging this even though it's a small
improvement. There isn't really that much code here, even though there is a
Pyrex module, so I'm happy for this to be merged as a modest incremental
improvement.
As far as the code itself, aside from John's remarks, I only have a couple of
remarks to make.
Robert Collins wrote:
[...]
> === added file 'bzrlib/_readdir_pyx.pyx'
[...]
> + if not (name[0] == dot and (
> + (name[1] == 0) or
> + (name[1] == dot and name [2] == 0))
There's a whitespace nit in “name [2]”.
> === modified file 'bzrlib/tests/test_osutils.py'
[...]
> +def load_tests(standard_tests, module, loader):
> + """Parameterize readdir tests."""
> + to_adapt, result = split_suite_by_re(standard_tests, "readdir")
> + adapter = TestScenarioApplier()
> + from bzrlib import _readdir_py
> + adapter.scenarios = [('python', {'read_dir':_readdir_py.read_dir})]
PEP 8 implies that colons should have whitespace after them (and most Python
code I've seen does so).
[...]
> + def test_readdir(self):
> + tree = [
> + '.bzr/',
> + '0file',
> + '1dir/',
> + '1dir/0file',
> + '1dir/1dir/',
> + '2file'
> + ]
> + self.build_tree(tree)
> + expected_names = ['.bzr', '0file', '1dir', '2file']
> + # read_dir either returns None, or a value
> + read_result = self.read_dir('.')
> + if read_result[0][0] is None:
> + # No innate sort:
> + expected_keys = [None, None, None, None]
> + expected = zip(expected_keys, expected_names)
> + expected.sort()
> + self.assertEqual(expected, sorted(read_result))
> + else:
> + # Check that the names we need are present in the second
> + # column
> + names = sorted([result[1] for result in read_result])
> + self.assertEqual(expected_names, names)
> +
If statements in test methods are a bad smell.
Why not just:
...
read_result = self.read_dir('.')
# Check that the names we need are present in the second
# column
names = sorted([result[1] for result in read_result])
self.assertEqual(expected_names, names)
If you separately want to assert that the _py version always fills in None as
the first value, then that strikes me as something that belongs in a separate
test (a test specifically for that implementation, rather than all read_dir
implementations).
-Andrew.
More information about the bazaar
mailing list