Code review comment for lp:~mbp/bzr/340394-encoding-option

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Martin Pool wrote:
> Martin Pool has proposed merging lp:~mbp/bzr/340394-encoding-option into lp:bzr.
>
> Requested reviews:
> bzr-core (bzr-core)
>
>
> This is probably a bit out of date with other test changes, but I'll put it up so bialix can see it: it adds an output_encoding config option that controls how we encode non-file-content output. My next step was going to be to add a way to set this on the command line for the duration of a process.
>

=== modified file 'bzrlib/tests/__init__.py'
- --- bzrlib/tests/__init__.py 2010-06-08 01:42:50 +0000
+++ bzrlib/tests/__init__.py 2010-06-18 09:45:47 +0000
@@ -3704,6 +3704,7 @@
         'bzrlib.tests.test_export',
         'bzrlib.tests.test_extract',
         'bzrlib.tests.test_fetch',
+ 'bzrlib.tests.test_fixtures',
         'bzrlib.tests.test_fifo_cache',
         'bzrlib.tests.test_filters',
         'bzrlib.tests.test_ftp_transport',
@@ -3839,6 +3840,7 @@
         'bzrlib.option',
         'bzrlib.symbol_versioning',
         'bzrlib.tests',
+ 'bzrlib.tests.fixtures',
         'bzrlib.timestamp',
         'bzrlib.version_info_formats.format_custom',
         ]

^- This confused me at first, but I see that it is "tests.test_fixtures"
versus "tests.fixtures". Probably fine overall, just mentioning the
confusion.

=== added file 'bzrlib/tests/test_fixtures.py'
- --- bzrlib/tests/test_fixtures.py 1970-01-01 00:00:00 +0000
+++ bzrlib/tests/test_fixtures.py 2010-06-18 09:45:47 +0000
@@ -0,0 +1,28 @@
+# Copyright (C) 2005-2010 Canonical Ltd

^- I'm pretty sure this isn't accurate :).

+class TestUIConfiguration(tests.TestCaseWithTransport):
+
+ def test_output_encoding_configuration(self):
+ enc = fixtures.generate_unicode_encodings().next()
+ config.GlobalConfig().set_user_option('output_encoding',
+ enc)
+ ui = tests.TestUIFactory(stdin=None,
+ stdout=tests.StringIOWrapper(),
+ stderr=tests.StringIOWrapper())
+ os = ui.make_output_stream()
+ self.assertEquals(os.encoding, enc)
+
+

^- this seems like it would want to be a permutation test across all the
'generate_unicode_encodings()' rather than a single test of the first one.

Conceptually I'm fine with this. I think it is a reasonable way to
handle stuff like "I want utf-8 always, even though my system doesn't
think so."

 merge: approve

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkwbgo4ACgkQJdeBCYSNAANGdQCghkA48qKjGt9reS13FkE8e2bK
ydAAoJZYizvZDCtdcKauNFwhfss15XTn
=4eZG
-----END PGP SIGNATURE-----