Checkbox

Merge lp:~pwlars/checkbox/impl-unit-flakes into lp:checkbox

  1. impl-unit-flakes
  2. Merge into trunk
Proposed by Paul Larson
Status: Merged
Approved by: Paul Larson
Approved revision: 4367
Merged at revision: 4373
Proposed branch: lp:~pwlars/checkbox/impl-unit-flakes
Merge into: lp:checkbox
Diff against target: 278 lines (+40/-44)
9 files modified
plainbox/plainbox/impl/unit/packaging.py (+15/-15)
plainbox/plainbox/impl/unit/test_category.py (+0/-1)
plainbox/plainbox/impl/unit/test_exporter.py (+0/-1)
plainbox/plainbox/impl/unit/test_job.py (+0/-2)
plainbox/plainbox/impl/unit/test_template.py (+0/-2)
plainbox/plainbox/impl/unit/test_unit.py (+0/-3)
plainbox/plainbox/impl/unit/testplan.py (+12/-7)
plainbox/plainbox/impl/unit/unit.py (+4/-4)
plainbox/plainbox/impl/unit/validators.py (+9/-9)
To merge this branch: bzr merge lp:~pwlars/checkbox/impl-unit-flakes
Reviewer Review Type Date Requested Status
Taihsiang Ho Approve
Paul Larson Needs Resubmitting
Sylvain Pineau Needs Fixing
Review via email: mp+295269@code.launchpad.net

Description of the change

More cleanups, this time on plainbox/plainbox/impl/unit
Also, doctests are annoying... ugly, but they work still

To post a comment you must log in.
Revision history for this message
Sylvain Pineau (sylvain-pineau) wrote :

Cleanup looks good, thanks but...

Your branch does not only include flake cleanups, there's also a fix for https://bugs.launchpad.net/plainbox/+bug/1473941.

Two possibilities, another branch or a dedicated commit.

review: Needs Fixing
Revision history for this message
Paul Larson (pwlars) wrote :

> Cleanup looks good, thanks but...
>
> Your branch does not only include flake cleanups, there's also a fix for
> https://bugs.launchpad.net/plainbox/+bug/1473941.
>
> Two possibilities, another branch or a dedicated commit.
Good catch! I was looking at that and didn't realize I had made the change in that same branch. Thanks for spotting it. I'll remove it and repush in a bit. It's an incomplete fix too I think. I'd like to look at fixing it, but want to better understand how to reproduce so that I can make sure the fix is right... but that belongs in another mp for sure.

lp:~pwlars/checkbox/impl-unit-flakes updated
4367. By Paul Larson

Cleanup some flakes in plainbox/plainbox/impl/unit

Revision history for this message
Paul Larson (pwlars) wrote :

Updated, thanks!

review: Needs Resubmitting
Revision history for this message
Taihsiang Ho (tai271828) wrote :

looks good to me.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plainbox/plainbox/impl/unit/packaging.py'
2--- plainbox/plainbox/impl/unit/packaging.py 2015-09-25 07:36:02 +0000
3+++ plainbox/plainbox/impl/unit/packaging.py 2016-05-20 13:55:26 +0000
4@@ -299,10 +299,10 @@
5 _logger.debug(_("Considering strategy: %s"),
6 _("os-id == ID and os-version-id == VERSION_ID"))
7 return (
8- 'ID' in os_release
9- and unit.os_id == os_release['ID']
10- and 'VERSION_ID' in os_release
11- and unit.os_version_id == os_release['VERSION_ID']
12+ 'ID' in os_release and
13+ unit.os_id == os_release['ID'] and
14+ 'VERSION_ID' in os_release and
15+ unit.os_version_id == os_release['VERSION_ID']
16 )
17
18
19@@ -310,9 +310,9 @@
20 _logger.debug(_("Considering strategy: %s"),
21 _("os-id == ID and os-version-id == undefined"))
22 return (
23- 'ID' in os_release
24- and unit.os_id == os_release['ID']
25- and unit.os_version_id is None
26+ 'ID' in os_release and
27+ unit.os_id == os_release['ID'] and
28+ unit.os_version_id is None
29 )
30
31
32@@ -320,9 +320,9 @@
33 _logger.debug(_("Considering strategy: %s"),
34 _("os-id == ID_LIKE and os-version-id == undefined"))
35 return (
36- 'ID_LIKE' in os_release
37- and unit.os_id == os_release['ID_LIKE']
38- and unit.os_version_id is None
39+ 'ID_LIKE' in os_release and
40+ unit.os_id == os_release['ID_LIKE'] and
41+ unit.os_version_id is None
42 )
43
44
45@@ -337,9 +337,9 @@
46 os_release = self.os_release
47 if unit.Meta.name != PackagingMetaDataUnit.Meta.name:
48 return False
49- if (not _strategy_id_version(unit, os_release)
50- and not _strategy_id(unit, os_release)
51- and not _strategy_id_like(unit, os_release)):
52+ if (not _strategy_id_version(unit, os_release) and not
53+ _strategy_id(unit, os_release) and not
54+ _strategy_id_like(unit, os_release)):
55 _logger.debug(_("All strategies unsuccessful"))
56 return False
57 _logger.debug(_("Last strategy was successful"))
58@@ -460,8 +460,8 @@
59 """Get the packaging driver appropriate for the current platform."""
60 if sys.platform.startswith("linux"):
61 os_release = get_os_release()
62- if (os_release.get('ID') == 'debian'
63- or os_release.get('ID_LIKE') == 'debian'):
64+ if (os_release.get('ID') == 'debian' or
65+ os_release.get('ID_LIKE') == 'debian'):
66 _logger.info(_("Using Debian packaging driver"))
67 return DebianPackagingDriver(os_release)
68 _logger.info(_("Using null packaging driver"))
69
70=== modified file 'plainbox/plainbox/impl/unit/test_category.py'
71--- plainbox/plainbox/impl/unit/test_category.py 2016-05-11 14:36:13 +0000
72+++ plainbox/plainbox/impl/unit/test_category.py 2016-05-20 13:55:26 +0000
73@@ -33,7 +33,6 @@
74 from plainbox.impl.unit.test_unit_with_id import UnitWithIdFieldValidationTests
75 from plainbox.impl.validation import Problem
76 from plainbox.impl.validation import Severity
77-from plainbox.impl.validation import ValidationError
78 from plainbox.vendor import mock
79
80
81
82=== modified file 'plainbox/plainbox/impl/unit/test_exporter.py'
83--- plainbox/plainbox/impl/unit/test_exporter.py 2016-05-11 14:36:13 +0000
84+++ plainbox/plainbox/impl/unit/test_exporter.py 2016-05-20 13:55:26 +0000
85@@ -34,7 +34,6 @@
86 from plainbox.impl.unit.test_unit_with_id import UnitWithIdFieldValidationTests
87 from plainbox.impl.validation import Problem
88 from plainbox.impl.validation import Severity
89-from plainbox.impl.validation import ValidationError
90 from plainbox.vendor import mock
91
92
93
94=== modified file 'plainbox/plainbox/impl/unit/test_job.py'
95--- plainbox/plainbox/impl/unit/test_job.py 2016-05-11 14:36:13 +0000
96+++ plainbox/plainbox/impl/unit/test_job.py 2016-05-20 13:55:26 +0000
97@@ -25,7 +25,6 @@
98 """
99
100 from unittest import TestCase
101-import warnings
102
103 from plainbox.impl.providers.v1 import Provider1
104 from plainbox.impl.secure.origin import FileTextSource
105@@ -38,7 +37,6 @@
106 from plainbox.impl.unit.validators import UnitValidationContext
107 from plainbox.impl.validation import Problem
108 from plainbox.impl.validation import Severity
109-from plainbox.impl.validation import ValidationError
110 from plainbox.testing_utils.testcases import TestCaseWithParameters
111 from plainbox.vendor import mock
112
113
114=== modified file 'plainbox/plainbox/impl/unit/test_template.py'
115--- plainbox/plainbox/impl/unit/test_template.py 2016-05-11 14:36:13 +0000
116+++ plainbox/plainbox/impl/unit/test_template.py 2016-05-20 13:55:26 +0000
117@@ -24,7 +24,6 @@
118 """
119
120 from unittest import TestCase
121-import warnings
122
123 from plainbox.abc import IProvider1
124 from plainbox.impl.resource import Resource
125@@ -37,7 +36,6 @@
126 from plainbox.impl.unit.validators import UnitValidationContext
127 from plainbox.impl.validation import Problem
128 from plainbox.impl.validation import Severity
129-from plainbox.impl.validation import ValidationError
130 from plainbox.vendor import mock
131
132
133
134=== modified file 'plainbox/plainbox/impl/unit/test_unit.py'
135--- plainbox/plainbox/impl/unit/test_unit.py 2016-05-11 14:36:13 +0000
136+++ plainbox/plainbox/impl/unit/test_unit.py 2016-05-20 13:55:26 +0000
137@@ -24,14 +24,11 @@
138 """
139
140 from unittest import TestCase
141-import warnings
142
143 from plainbox.abc import IProvider1
144 from plainbox.impl.unit.unit import Unit
145-from plainbox.impl.unit.unit_with_id import UnitWithId
146 from plainbox.impl.validation import Problem
147 from plainbox.impl.validation import Severity
148-from plainbox.impl.validation import ValidationError
149 from plainbox.vendor import mock
150
151
152
153=== modified file 'plainbox/plainbox/impl/unit/testplan.py'
154--- plainbox/plainbox/impl/unit/testplan.py 2016-05-11 14:36:13 +0000
155+++ plainbox/plainbox/impl/unit/testplan.py 2016-05-20 13:55:26 +0000
156@@ -337,7 +337,8 @@
157 the include and exclude fields.
158 """
159 qual_list = []
160- qual_list.extend(self._gen_qualifiers('include', self.mandatory_include, True))
161+ qual_list.extend(
162+ self._gen_qualifiers('include', self.mandatory_include, True))
163 return CompositeQualifier(qual_list)
164
165 def get_bootstrap_qualifier(self, excluding=False):
166@@ -611,8 +612,8 @@
167 lambda duration, unit: unit.estimated_duration > 0,
168 message="value must be a positive number",
169 onlyif=lambda unit: (
170- unit.virtual is False
171- and unit.get_record_value('estimated_duration'))),
172+ unit.virtual is False and
173+ unit.get_record_value('estimated_duration'))),
174 ],
175 fields.icon: [
176 UntranslatableFieldValidator,
177@@ -677,10 +678,14 @@
178 And the qualifiers:
179
180 >>> support.qualifier # doctest: +NORMALIZE_WHITESPACE
181- CompositeQualifier(qualifier_list=[FieldQualifier('id', OperatorMatcher(<built-in function eq>, 'job-a'), inclusive=True),
182- FieldQualifier('id', OperatorMatcher(<built-in function eq>, 'job-b'), inclusive=True),
183- FieldQualifier('id', OperatorMatcher(<built-in function eq>, 'job-c'), inclusive=True),
184- FieldQualifier('id', PatternMatcher('^job-[x-z]$'), inclusive=False)])
185+ CompositeQualifier(qualifier_list=[FieldQualifier('id', \
186+OperatorMatcher(<built-in function eq>, 'job-a'), inclusive=True),
187+ FieldQualifier('id', \
188+OperatorMatcher(<built-in function eq>, 'job-b'), inclusive=True),
189+ FieldQualifier('id', \
190+OperatorMatcher(<built-in function eq>, 'job-c'), inclusive=True),
191+ FieldQualifier('id', \
192+PatternMatcher('^job-[x-z]$'), inclusive=False)])
193 """
194
195 def __init__(self, testplan):
196
197=== modified file 'plainbox/plainbox/impl/unit/unit.py'
198--- plainbox/plainbox/impl/unit/unit.py 2016-05-11 14:36:13 +0000
199+++ plainbox/plainbox/impl/unit/unit.py 2016-05-20 13:55:26 +0000
200@@ -207,8 +207,8 @@
201 elif '_{}'.format(field) in unit[0].field_offset_map:
202 if origin is None:
203 origin = origin.with_offset(
204- unit[0].field_offset_map['_{}'.format(field)]
205- + offset).just_line()
206+ unit[0].field_offset_map['_{}'.format(field)] +
207+ offset).just_line()
208 else:
209 cls = UnitFieldIssue
210 if origin is None:
211@@ -220,8 +220,8 @@
212 elif '_{}'.format(field) in unit.field_offset_map:
213 if origin is None:
214 origin = origin.with_offset(
215- unit.field_offset_map['_{}'.format(field)]
216- + offset).just_line()
217+ unit.field_offset_map['_{}'.format(field)] +
218+ offset).just_line()
219 issue = cls(message, severity, kind, origin, unit, field)
220 self.issue_list.append(issue)
221 return issue
222
223=== modified file 'plainbox/plainbox/impl/unit/validators.py'
224--- plainbox/plainbox/impl/unit/validators.py 2015-09-03 11:21:13 +0000
225+++ plainbox/plainbox/impl/unit/validators.py 2016-05-20 13:55:26 +0000
226@@ -329,7 +329,7 @@
227 issue if the validation fails. By default it is derived from the
228 specified issue ``kind`` by :meth:`UnitValidator.explain()`.
229 """
230- correct_fn = lambda value: value is not None
231+ def correct_fn(value): return value is not None
232 super().__init__(correct_fn, kind, severity, message, onlyif)
233
234
235@@ -360,7 +360,7 @@
236 issue if the validation fails. By default it is derived from the
237 specified issue ``kind`` by :meth:`UnitValidator.explain()`.
238 """
239- correct_fn = lambda value: value is None
240+ def correct_fn(value): return value is None
241 super().__init__(correct_fn, kind, severity, message, onlyif)
242
243
244@@ -390,9 +390,9 @@
245 """
246
247 def check(self, parent, unit, field):
248- if (unit.virtual is False
249- and unit.get_record_value(field) is not None
250- and not unit.is_translatable_field(field)):
251+ if (unit.virtual is False and
252+ unit.get_record_value(field) is not None and not
253+ unit.is_translatable_field(field)):
254 yield parent.warning(unit, field, Problem.expected_i18n)
255
256
257@@ -409,8 +409,8 @@
258 """
259
260 def check(self, parent, unit, field):
261- if (unit.get_record_value(field)
262- and unit.is_translatable_field(field)):
263+ if (unit.get_record_value(field) and
264+ unit.is_translatable_field(field)):
265 yield parent.warning(unit, field, Problem.unexpected_i18n)
266
267
268@@ -639,8 +639,8 @@
269 if not constraint.constraint_fn(referrer, referee):
270 yield parent.error(
271 unit, field, Problem.bad_reference,
272- self.message or constraint.message
273- or _("referee constraint failed"))
274+ self.message or constraint.message or
275+ _("referee constraint failed"))
276 elif n > 1:
277 # more than one is also good, which one are we targeting?
278 yield parent.error(

Subscribers

People subscribed via source and target branches