Merge lp:~diegosarmentero/ubuntuone-dev-tools/temp-bytes into lp:ubuntuone-dev-tools

Proposed by Diego Sarmentero on 2012-09-28
Status: Merged
Approved by: Diego Sarmentero on 2012-09-28
Approved revision: 103
Merged at revision: 97
Proposed branch: lp:~diegosarmentero/ubuntuone-dev-tools/temp-bytes
Merge into: lp:ubuntuone-dev-tools
Diff against target: 63 lines (+41/-1)
3 files modified
ubuntuone/devtools/runners/__init__.py (+1/-1)
ubuntuone/devtools/runners/tests/__init__.py (+1/-0)
ubuntuone/devtools/runners/tests/test_base.py (+39/-0)
To merge this branch: bzr merge lp:~diegosarmentero/ubuntuone-dev-tools/temp-bytes
Reviewer Review Type Date Requested Status
dobey (community) 2012-09-28 Approve on 2012-09-28
Review via email: mp+126973@code.launchpad.net

Commit message

- Making temp-directory to be bytes to avoid encoding problems in tests.

To post a comment you must log in.
dobey (dobey) wrote :

Can you please file a bug for this (temp directory being unicode when it should be bytes), and add a regression test so that we can ensure it doesn't pop up again? Thanks.

review: Needs Fixing
98. By Diego Sarmentero on 2012-09-28

Adding regression test

99. By Diego Sarmentero on 2012-09-28

fixing docstring

100. By Diego Sarmentero on 2012-09-28

fixing import order

dobey (dobey) wrote :

62 + def test_check_temp_directory_is_bytes(self):
63 + """Check that the temp-directory value is bytes."""
64 + base_options = runners.BaseTestOptions()
65 + self.assertIsInstance(base_options.optParameters[5][2], str)

Instead of poking at this directly, I think it's probably better to perhaps check that self.tempdir.decode('utf-8') succeeds. If the parameters list changes in the future, your test will break, even when it shouldn't, as it depends on the order of the parameters. Also, I think your test will fail on python3, as the instance of that parameter should be 'bytes' instead of 'str' there at least. It's possible we may need to check the Python version and do things differently on 2 and 3 in the test, as well, but I think the .decode test should be enough for now.

101. By Diego Sarmentero on 2012-09-28

improve tests

102. By Diego Sarmentero on 2012-09-28

updating test

103. By Diego Sarmentero on 2012-09-28

removing unncessary lint ignore

dobey (dobey) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'ubuntuone/devtools/runners/__init__.py'
2--- ubuntuone/devtools/runners/__init__.py 2012-08-27 19:15:04 +0000
3+++ ubuntuone/devtools/runners/__init__.py 2012-09-28 17:48:20 +0000
4@@ -198,7 +198,7 @@
5 ['ignore-modules', 'i', '', None],
6 ['ignore-paths', 'p', '', None],
7 ['runner', None, 'txrunner', None],
8- ['temp-directory', None, '_trial_temp', None],
9+ ['temp-directory', None, b'_trial_temp', None],
10 ]
11
12 def __init__(self, *args, **kwargs):
13
14=== added directory 'ubuntuone/devtools/runners/tests'
15=== added file 'ubuntuone/devtools/runners/tests/__init__.py'
16--- ubuntuone/devtools/runners/tests/__init__.py 1970-01-01 00:00:00 +0000
17+++ ubuntuone/devtools/runners/tests/__init__.py 2012-09-28 17:48:20 +0000
18@@ -0,0 +1,1 @@
19+"""Test the dev tools."""
20
21=== added file 'ubuntuone/devtools/runners/tests/test_base.py'
22--- ubuntuone/devtools/runners/tests/test_base.py 1970-01-01 00:00:00 +0000
23+++ ubuntuone/devtools/runners/tests/test_base.py 2012-09-28 17:48:20 +0000
24@@ -0,0 +1,39 @@
25+# -*- coding: utf-8 *-*
26+#
27+# Copyright 2012 Canonical Ltd.
28+#
29+# This program is free software: you can redistribute it and/or modify it
30+# under the terms of the GNU General Public License version 3, as published
31+# by the Free Software Foundation.
32+#
33+# This program is distributed in the hope that it will be useful, but
34+# WITHOUT ANY WARRANTY; without even the implied warranties of
35+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
36+# PURPOSE. See the GNU General Public License for more details.
37+#
38+# You should have received a copy of the GNU General Public License along
39+# with this program. If not, see <http://www.gnu.org/licenses/>.
40+#
41+# In addition, as a special exception, the copyright holders give
42+# permission to link the code of portions of this program with the
43+# OpenSSL library under certain conditions as described in each
44+# individual source file, and distribute linked combinations
45+# including the two.
46+# You must obey the GNU General Public License in all respects
47+# for all of the code used other than OpenSSL. If you modify
48+# file(s) with this exception, you may extend this exception to your
49+# version of the file(s), but you are not obligated to do so. If you
50+# do not wish to do so, delete this exception statement from your
51+# version. If you delete this exception statement from all source
52+# files in the program, then also delete it here.
53+"""Test the base module."""
54+
55+from ubuntuone.devtools.testcases import BaseTestCase
56+
57+
58+class BaseRunnersTestCase(BaseTestCase):
59+ """Test the runners module."""
60+
61+ def test_check_temp_directory_is_bytes(self):
62+ """Check that the temp directory value is bytes."""
63+ self.assertIsInstance(self.tmpdir, bytes)

Subscribers

People subscribed via source and target branches

to all changes: