Merge ~emitorino/review-tools:multi_value_attrs_in_decl into review-tools:master
- Git
- lp:~emitorino/review-tools
- multi_value_attrs_in_decl
- Merge into master
Status: | Merged | ||||
---|---|---|---|---|---|
Merged at revision: | 8f1d06564faf23c9ddfef696007bc871715ec79a | ||||
Proposed branch: | ~emitorino/review-tools:multi_value_attrs_in_decl | ||||
Merge into: | review-tools:master | ||||
Diff against target: |
568 lines (+448/-66) 2 files modified
reviewtools/sr_declaration.py (+49/-63) reviewtools/tests/test_sr_declaration.py (+399/-3) |
||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Samuele Pedroni (community) | Approve | ||
Alex Murray | Approve | ||
Review via email: mp+427054@code.launchpad.net |
Commit message
- sr_declaration.py: add support for multi value attrs declarations
- test_sr_
Description of the change
The current code considers if the attribute in the constraint is a list, just one must match (it is
considered a list of alternatives, aka, a list of OR constraints).
As per https:/
Whats pending to do is to confirm if nested list attrs should be allowed. We can create a follow up MP to include it.
I added several test scenarios including the cases I could think of that should/should not be allowed after this change. Please suggest more that I could be missing so we make sure we dont break this important functionality.
The current test_sr_
Samuele Pedroni (pedronis) wrote : | # |
question about more deep/recursive situations
Emilia Torino (emitorino) wrote : | # |
Adding support for nested lists in declaration attrs
Samuele Pedroni (pedronis) wrote : | # |
lgtm, wondering if the code can be simplified now though, as the code for lists _chec_attrib and the top loop do similar things ?
Emilia Torino (emitorino) wrote : | # |
> lgtm, wondering if the code can be simplified now though, as the code for
> lists _chec_attrib and the top loop do similar things ?
Thanks for this comment. I indeed simplified the rules processing code but had to extend a bit more the _chec_attrib to support considering lists in "against" (rules) for non-lists in val (snap decl).
Preview Diff
1 | diff --git a/reviewtools/sr_declaration.py b/reviewtools/sr_declaration.py |
2 | index 58f5f3b..61bd313 100644 |
3 | --- a/reviewtools/sr_declaration.py |
4 | +++ b/reviewtools/sr_declaration.py |
5 | @@ -811,61 +811,49 @@ class SnapReviewDeclaration(SnapReview): |
6 | if type(val) not in [str, list, dict, bool]: |
7 | raise SnapDeclarationException("unknown type '%s'" % val) |
8 | |
9 | - # Keep in mind by this point each OR constraint is being iterated |
10 | - # through such that 'against' as a list is a nested list. |
11 | - # For now, punt on nested lists since they are impractical in use |
12 | - # since they are a list of OR options where one option must match |
13 | - # all of val, but if you have that you may as well just use a |
14 | - # string. Eg for this snap.yaml: |
15 | - # snap.yaml: |
16 | - # plugs: |
17 | - # foo: |
18 | - # bar: [ baz, norf ] |
19 | - # |
20 | - # a snap decl that uses a list for 'bar' might be: |
21 | - # snap decl: |
22 | - # foo: |
23 | - # plug-attributes: |
24 | - # bar: |
25 | - # - baz|norf |
26 | - # - something|else |
27 | - # |
28 | - # but, 'something|else' is pointless so it will never match, so the |
29 | - # decl should be rewritten more simply as: |
30 | - # snap decl: |
31 | - # foo: |
32 | - # plug-attributes: |
33 | - # bar: baz|norf |
34 | - # Importantly, the type of the attribute in the snap decl means |
35 | - # something different than the type in snap.yaml |
36 | - if isinstance(against, list): |
37 | - raise SnapDeclarationException( |
38 | - "attribute lists in the declaration not supported" |
39 | - ) |
40 | - |
41 | matched = False |
42 | - if isinstance(val, str) and isinstance(against, str): |
43 | - if against.startswith("$"): |
44 | - if against == "$MISSING": |
45 | - matched = False # value must not be set |
46 | - elif re.search(r"^\$PLUG\(%s\)$" % rules_attrib, against): |
47 | - matched = True |
48 | - elif re.search(r"^\$SLOT\(%s\)$" % rules_attrib, against): |
49 | + if isinstance(val, str): |
50 | + if isinstance(against, str): |
51 | + if against.startswith("$"): |
52 | + if against == "$MISSING": |
53 | + matched = False # value must not be set |
54 | + elif re.search(r"^\$PLUG\(%s\)$" % rules_attrib, against): |
55 | + matched = True |
56 | + elif re.search(r"^\$SLOT\(%s\)$" % rules_attrib, against): |
57 | + matched = True |
58 | + else: |
59 | + raise SnapDeclarationException( |
60 | + "unknown special attrib '%s'" % against |
61 | + ) |
62 | + elif re.search(r"^(%s)$" % against, val): |
63 | matched = True |
64 | - else: |
65 | - raise SnapDeclarationException( |
66 | - "unknown special attrib '%s'" % against |
67 | - ) |
68 | - elif re.search(r"^(%s)$" % against, val): |
69 | - matched = True |
70 | + elif isinstance(against, list): |
71 | + for declaration_value in against: |
72 | + # if the attribute in the snap (val) is a string and the |
73 | + # declaration value (against) is a list, then to match, |
74 | + # val must match some entry in against |
75 | + if _check_attrib(val, declaration_value, side, rules_attrib): |
76 | + matched = True |
77 | + break |
78 | elif isinstance(val, list): |
79 | - # if the attribute in the snap (val) is a list and the |
80 | - # declaration value (against) is a string, then to match, |
81 | - # against must be a regex that matches all entries in val |
82 | num_matched = 0 |
83 | - for i in val: |
84 | - if _check_attrib(i, against, side, rules_attrib): |
85 | - num_matched += 1 |
86 | + for snap_attribute in val: |
87 | + if isinstance(against, str): |
88 | + # if the attribute in the snap (val) is a list and the |
89 | + # declaration value (against) is a string, then to match, |
90 | + # against must be a regex that matches all entries in val |
91 | + if _check_attrib(snap_attribute, against, side, rules_attrib): |
92 | + num_matched += 1 |
93 | + elif isinstance(against, list): |
94 | + for declaration_value in against: |
95 | + # if the attribute in the snap (val) is a list and the |
96 | + # declaration value (against) is a list, then to match, |
97 | + # every item in against must match some entry in val |
98 | + if _check_attrib( |
99 | + snap_attribute, declaration_value, side, rules_attrib |
100 | + ): |
101 | + num_matched += 1 |
102 | + break |
103 | if num_matched == len(val): |
104 | matched = True |
105 | elif isinstance(val, dict): |
106 | @@ -880,6 +868,14 @@ class SnapReviewDeclaration(SnapReview): |
107 | ) |
108 | if not matched: |
109 | break |
110 | + elif isinstance(against, list): |
111 | + for declaration_value in against: |
112 | + # if the attribute in the snap (val) is a dict and the |
113 | + # declaration value (against) is a list, then to match, |
114 | + # val must match some entry in against |
115 | + if _check_attrib(val, declaration_value, side, rules_attrib): |
116 | + matched = True |
117 | + break |
118 | else: |
119 | matched = False |
120 | else: # bools |
121 | @@ -973,18 +969,8 @@ class SnapReviewDeclaration(SnapReview): |
122 | val = iface[rules_attrib] |
123 | against = rules[rules_key][rules_attrib] |
124 | |
125 | - if isinstance(against, list): |
126 | - # As a practical matter, if the attribute in the |
127 | - # constraint is a list, just one must match (it is |
128 | - # considered a list of alternatives, aka, a list of |
129 | - # OR constraints). |
130 | - for i in against: |
131 | - if _check_attrib(val, i, side, rules_attrib): |
132 | - attributes_matched[rules_key]["matched"] += 1 |
133 | - break |
134 | - else: |
135 | - if _check_attrib(val, against, side, rules_attrib): |
136 | - attributes_matched[rules_key]["matched"] += 1 |
137 | + if _check_attrib(val, against, side, rules_attrib): |
138 | + attributes_matched[rules_key]["matched"] += 1 |
139 | else: |
140 | # when the attribute is missing from the interface don't |
141 | # mark as checked (missing attributes are checked |
142 | diff --git a/reviewtools/tests/test_sr_declaration.py b/reviewtools/tests/test_sr_declaration.py |
143 | index 1b36992..38154f0 100644 |
144 | --- a/reviewtools/tests/test_sr_declaration.py |
145 | +++ b/reviewtools/tests/test_sr_declaration.py |
146 | @@ -3892,6 +3892,403 @@ slots: |
147 | } |
148 | self.check_results(r, expected=expected) |
149 | |
150 | + def test_check_declaration_slots_multi_value_attrs_issue(self,): |
151 | + review_ok_output = { |
152 | + "error": dict(), |
153 | + "warn": dict(), |
154 | + "info": { |
155 | + "declaration-snap-v2:slots:fram-devices:custom-device": {"text": "OK"} |
156 | + }, |
157 | + } |
158 | + |
159 | + review_err_output = { |
160 | + "error": { |
161 | + "declaration-snap-v2:slots_installation:fram-devices:custom-device": { |
162 | + "text": "human review required due to 'allow-installation' constraint (interface attributes)" |
163 | + } |
164 | + }, |
165 | + "warn": dict(), |
166 | + "info": dict(), |
167 | + } |
168 | + |
169 | + multi_value_attrs_test_data = { |
170 | + "iface_with_multiple_multi_value_attrs_and_multi_value_decl_matches": ( |
171 | + { |
172 | + "read-devices": [ |
173 | + "/dev/chunkfs/geom", |
174 | + "/dev/chunkfs/stats", |
175 | + "/dev/chunkfs/sizes", |
176 | + ], |
177 | + "devices": ["/dev/mtd[0-9]", "/dev/chunkfs/chunkfs[0-2]"], |
178 | + }, |
179 | + { |
180 | + "read-devices": [ |
181 | + "/dev/chunkfs/geom", |
182 | + "/dev/chunkfs/stats", |
183 | + "/dev/chunkfs/sizes", |
184 | + ], |
185 | + "devices": ["/dev/mtd\\[0-9\\]", "/dev/chunkfs/chunkfs\\[0-2\\]"], |
186 | + }, |
187 | + {"info": 1, "warn": 0, "error": 0}, |
188 | + review_ok_output, |
189 | + ), |
190 | + "iface_with_multiple_multi_value_attrs_and_multi_value_decl_different_order_matches": ( |
191 | + { |
192 | + "read-devices": [ |
193 | + "/dev/chunkfs/stats", |
194 | + "/dev/chunkfs/sizes", |
195 | + "/dev/chunkfs/geom", |
196 | + ], |
197 | + "devices": ["/dev/chunkfs/chunkfs[0-2]", "/dev/mtd[0-9]"], |
198 | + }, |
199 | + { |
200 | + "read-devices": [ |
201 | + "/dev/chunkfs/geom", |
202 | + "/dev/chunkfs/stats", |
203 | + "/dev/chunkfs/sizes", |
204 | + ], |
205 | + "devices": ["/dev/mtd\\[0-9\\]", "/dev/chunkfs/chunkfs\\[0-2\\]"], |
206 | + }, |
207 | + {"info": 1, "warn": 0, "error": 0}, |
208 | + review_ok_output, |
209 | + ), |
210 | + "iface_with_multiple_multi_value_attrs_and_single_value_list_decl_matches": ( |
211 | + {"devices": ["/dev/mtd1", "/dev/mtd2"]}, |
212 | + {"devices": ["/dev/mtd[0-9]"]}, |
213 | + {"info": 1, "warn": 0, "error": 0}, |
214 | + review_ok_output, |
215 | + ), |
216 | + "iface_with_single_value_attrs_and_single_value_list_decl_matches": ( |
217 | + {"devices": "/dev/mtd1"}, |
218 | + {"devices": ["/dev/mtd[0-9]"]}, |
219 | + {"info": 1, "warn": 0, "error": 0}, |
220 | + review_ok_output, |
221 | + ), |
222 | + "iface_with_single_value_attrs_and_single_value_decl_matches": ( |
223 | + {"devices": "/dev/mtd1"}, |
224 | + {"devices": "/dev/mtd[0-9]"}, |
225 | + {"info": 1, "warn": 0, "error": 0}, |
226 | + review_ok_output, |
227 | + ), |
228 | + "iface_with_one_multi_value_attrs_all_matching": ( |
229 | + {"devices": ["/dev/mtd[0-9]", "/dev/chunkfs/chunkfs[0-2]"]}, |
230 | + {"devices": ["/dev/mtd\\[0-9\\]", "/dev/chunkfs/chunkfs\\[0-2\\]"]}, |
231 | + {"info": 1, "warn": 0, "error": 0}, |
232 | + review_ok_output, |
233 | + ), |
234 | + "regex_in_list_of_single_attr_matches": ( # This case represents the current workaround |
235 | + { |
236 | + "read-devices": [ |
237 | + "/dev/chunkfs/geom", |
238 | + "/dev/chunkfs/stats", |
239 | + "/dev/chunkfs/sizes", |
240 | + ], |
241 | + "devices": ["/dev/mtd[0-9]", "/dev/chunkfs/chunkfs[0-2]"], |
242 | + }, |
243 | + { |
244 | + "devices": ["(/dev/mtd\\[0-9\\]|/dev/chunkfs/chunkfs\\[0-2\\])"], |
245 | + "read-devices": [ |
246 | + "(/dev/chunkfs/geom|/dev/chunkfs/stats|/dev/chunkfs/sizes)" |
247 | + ], |
248 | + }, |
249 | + {"info": 1, "warn": 0, "error": 0}, |
250 | + review_ok_output, |
251 | + ), |
252 | + "extra_iface_attr_matches": ( # Even though this test passes here, |
253 | + # unknown attrs are detected as part _verify_declaration check |
254 | + { |
255 | + "read-devices": [ |
256 | + "/dev/chunkfs/geom", |
257 | + "/dev/chunkfs/stats", |
258 | + "/dev/chunkfs/sizes", |
259 | + ], |
260 | + "devices": ["/dev/mtd[0-9]", "/dev/chunkfs/chunkfs[0-2]"], |
261 | + }, |
262 | + { |
263 | + "read-devices": [ |
264 | + "/dev/chunkfs/geom", |
265 | + "/dev/chunkfs/stats", |
266 | + "/dev/chunkfs/sizes", |
267 | + ], |
268 | + }, |
269 | + {"info": 1, "warn": 0, "error": 0}, |
270 | + review_ok_output, |
271 | + ), |
272 | + "less_items_in_iface_attr_list_but_all_matching_matches": ( |
273 | + {"read-devices": ["/dev/chunkfs/geom", "/dev/chunkfs/stats"]}, |
274 | + { |
275 | + "read-devices": [ |
276 | + "/dev/chunkfs/geom", |
277 | + "/dev/chunkfs/stats", |
278 | + "/dev/chunkfs/sizes", |
279 | + ], |
280 | + }, |
281 | + {"info": 1, "warn": 0, "error": 0}, |
282 | + review_ok_output, |
283 | + ), |
284 | + "missing_attr_in_iface_fails": ( |
285 | + { |
286 | + "read-devices": [ |
287 | + "/dev/chunkfs/geom", |
288 | + "/dev/chunkfs/stats", |
289 | + "/dev/chunkfs/sizes", |
290 | + ], |
291 | + }, |
292 | + { |
293 | + "read-devices": [ |
294 | + "/dev/chunkfs/geom", |
295 | + "/dev/chunkfs/stats", |
296 | + "/dev/chunkfs/sizes", |
297 | + ], |
298 | + "devices": ["/dev/mtd\\[0-9\\]", "/dev/chunkfs/chunkfs\\[0-2\\]"], |
299 | + }, |
300 | + {"info": 0, "warn": 0, "error": 1}, |
301 | + review_err_output, |
302 | + ), |
303 | + "no_matching_item_in_iface_attr_list_fails": ( |
304 | + { |
305 | + "read-devices": [ |
306 | + "/dev/chunkfs/geom", |
307 | + "/dev/chunkfs/stats", |
308 | + "/dev/chunkfs/sizes", |
309 | + ] |
310 | + }, |
311 | + {"read-devices": ["/dev/chunkfs/geom", "/dev/chunkfs/stats"]}, |
312 | + {"info": 0, "warn": 0, "error": 1}, |
313 | + review_err_output, |
314 | + ), |
315 | + "iface_with_single_value_attrs_and_multiple_value_decl_matches": ( |
316 | + {"devices": "/dev/mtd1"}, |
317 | + {"devices": ["/dev/mtd[0-9]", "/dev/chunkfs/chunkfs[0-2]"]}, |
318 | + {"info": 1, "warn": 0, "error": 0}, |
319 | + review_ok_output, |
320 | + ), |
321 | + } |
322 | + |
323 | + for test, test_data in multi_value_attrs_test_data.items(): |
324 | + ( |
325 | + iface_attr_value, |
326 | + rule_value, |
327 | + expected_counts, |
328 | + expected_output_data, |
329 | + ) = test_data |
330 | + with self.subTest( |
331 | + iface_attr_value=iface_attr_value, |
332 | + rule_value=iface_attr_value, |
333 | + expected_counts=expected_counts, |
334 | + expected_output_data=expected_output_data, |
335 | + ): |
336 | + iface_data = {"interface": "custom-device"} |
337 | + iface_data.update(iface_attr_value) |
338 | + slots = {"fram-devices": iface_data} |
339 | + self.set_test_snap_yaml("slots", slots) |
340 | + c = SnapReviewDeclaration(self.test_name) |
341 | + base = { |
342 | + "slots": { |
343 | + "custom-device": { |
344 | + "allow-installation": { |
345 | + "slot-attributes": rule_value, |
346 | + "slot-names": ["fram-devices"], |
347 | + } |
348 | + } |
349 | + } |
350 | + } |
351 | + self._set_base_declaration(c, base) |
352 | + c.check_declaration() |
353 | + r = c.review_report |
354 | + self.check_results(r, expected_counts) |
355 | + self.check_results(r, expected=expected_output_data) |
356 | + |
357 | + def test_check_declaration_slots_multi_value_attrs_issue_support_nested(self,): |
358 | + review_ok_output = { |
359 | + "error": dict(), |
360 | + "warn": dict(), |
361 | + "info": {"declaration-snap-v2:plugs:mnt:mount-control": {"text": "OK"}}, |
362 | + } |
363 | + |
364 | + review_err_output = { |
365 | + "error": { |
366 | + "declaration-snap-v2:plugs_installation:mnt:mount-control": { |
367 | + "text": "human review required due to 'allow-installation' constraint (interface attributes)" |
368 | + } |
369 | + }, |
370 | + "warn": dict(), |
371 | + "info": dict(), |
372 | + } |
373 | + |
374 | + multi_value_attrs_test_data = { |
375 | + "iface_with_list_attrs_and_single_value_decl_list_matches": ( |
376 | + { |
377 | + "mnt": [ |
378 | + { |
379 | + "what": "/dev/x*", |
380 | + "where": "/foo/*", |
381 | + "options": ["rw", "nodev"], |
382 | + }, |
383 | + { |
384 | + "what": "/bar/*", |
385 | + "where": "/baz/*", |
386 | + "options": ["rw", "bind"], |
387 | + }, |
388 | + ] |
389 | + }, |
390 | + { |
391 | + "mnt": [ |
392 | + { |
393 | + "what": r"/(bar/|dev/x)\*", |
394 | + "where": r"/(foo|baz)/\*", |
395 | + "options": "rw|bind|nodev", |
396 | + } |
397 | + ] |
398 | + }, |
399 | + {"info": 1, "warn": 0, "error": 0}, |
400 | + review_ok_output, |
401 | + ), |
402 | + "iface_with_single_list_attrs_and_single_value_decl_list_matches": ( |
403 | + { |
404 | + "mnt": [ |
405 | + { |
406 | + "what": "/dev/x*", |
407 | + "where": "/foo/*", |
408 | + "options": ["rw", "nodev"], |
409 | + } |
410 | + ] |
411 | + }, |
412 | + { |
413 | + "mnt": [ |
414 | + { |
415 | + "what": r"/(bar/|dev/x)\*", |
416 | + "where": r"/(foo|baz)/\*", |
417 | + "options": "rw|bind|nodev", |
418 | + } |
419 | + ] |
420 | + }, |
421 | + {"info": 1, "warn": 0, "error": 0}, |
422 | + review_ok_output, |
423 | + ), |
424 | + "iface_with_list_attrs_and_multiple_value_decl_list_matches": ( |
425 | + { |
426 | + "mnt": [ |
427 | + { |
428 | + "what": "/dev/x*", |
429 | + "where": "/foo/*", |
430 | + "options": ["rw", "nodev"], |
431 | + }, |
432 | + { |
433 | + "what": "/bar/*", |
434 | + "where": "/baz/*", |
435 | + "options": ["rw", "bind"], |
436 | + }, |
437 | + ] |
438 | + }, |
439 | + { |
440 | + "mnt": [ |
441 | + { |
442 | + "what": r"/dev/x\*", |
443 | + "where": r"/foo/\*", |
444 | + "options": ["nodev", "rw"], |
445 | + }, |
446 | + { |
447 | + "what": r"/bar/\*", |
448 | + "where": r"/baz/\*", |
449 | + "options": ["rw", "bind"], |
450 | + }, |
451 | + ] |
452 | + }, |
453 | + {"info": 1, "warn": 0, "error": 0}, |
454 | + review_ok_output, |
455 | + ), |
456 | + "iface_with_list_attrs_and_multiple_value_decl_list_with_no_alternatives_fails": ( |
457 | + { |
458 | + "mnt": [ |
459 | + { |
460 | + "what": "/dev/x*", |
461 | + "where": "/foo/*", |
462 | + "options": ["rw", "nodev"], |
463 | + }, |
464 | + { |
465 | + "what": "/bar/*", |
466 | + "where": "/baz/*", |
467 | + "options": ["rw", "bind"], |
468 | + }, |
469 | + ] |
470 | + }, |
471 | + { |
472 | + "mnt": [ |
473 | + {"what": r"/dev/x\*", "where": r"/foo/\*", "options": ["rw"]}, |
474 | + { |
475 | + "what": r"/bar/\*", |
476 | + "where": r"/baz/\*", |
477 | + "options": ["rw", "bind"], |
478 | + }, |
479 | + ] |
480 | + }, |
481 | + {"info": 0, "warn": 0, "error": 1}, |
482 | + review_err_output, |
483 | + ), |
484 | + "iface_with_extra_list_attrs_and_multiple_value_decl_list_fails": ( |
485 | + { |
486 | + "mnt": [ |
487 | + { |
488 | + "what": "/dev/x*", |
489 | + "where": "/foo/*", |
490 | + "options": ["rw", "nodev"], |
491 | + }, |
492 | + { |
493 | + "what": "/bar/*", |
494 | + "where": "/baz/*", |
495 | + "foo_options": ["rw", "bind"], |
496 | + }, |
497 | + ] |
498 | + }, |
499 | + { |
500 | + "mnt": [ |
501 | + { |
502 | + "what": r"/(bar/|dev/x)\*", |
503 | + "where": r"/(foo|baz)/\*", |
504 | + "options": "rw|bind|nodev", |
505 | + } |
506 | + ] |
507 | + }, |
508 | + {"info": 0, "warn": 0, "error": 1}, |
509 | + review_err_output, |
510 | + ), |
511 | + } |
512 | + |
513 | + for test, test_data in multi_value_attrs_test_data.items(): |
514 | + ( |
515 | + iface_attr_value, |
516 | + rule_value, |
517 | + expected_counts, |
518 | + expected_output_data, |
519 | + ) = test_data |
520 | + with self.subTest( |
521 | + iface_attr_value=iface_attr_value, |
522 | + rule_value=iface_attr_value, |
523 | + expected_counts=expected_counts, |
524 | + expected_output_data=expected_output_data, |
525 | + ): |
526 | + iface_data = {"interface": "mount-control"} |
527 | + iface_data.update(iface_attr_value) |
528 | + plugs = {"mnt": iface_data} |
529 | + self.set_test_snap_yaml("plugs", plugs) |
530 | + c = SnapReviewDeclaration(self.test_name) |
531 | + base = { |
532 | + "plugs": { |
533 | + "mount-control": { |
534 | + "allow-installation": { |
535 | + "plug-attributes": rule_value, |
536 | + "plug-names": ["mnt"], |
537 | + } |
538 | + } |
539 | + } |
540 | + } |
541 | + self._set_base_declaration(c, base) |
542 | + c.check_declaration() |
543 | + r = c.review_report |
544 | + self.check_results(r, expected_counts) |
545 | + self.check_results(r, expected=expected_output_data) |
546 | + |
547 | def test_check_declaration_slots_deny_connection_attrib_list_match_unmatching_plug_names( |
548 | self, |
549 | ): |
550 | @@ -3973,7 +4370,7 @@ slots: |
551 | expected["info"][name] = {"text": "OK"} |
552 | self.check_results(r, expected=expected) |
553 | |
554 | - def test_check_declaration_slots_allow_connection_attrib_lists_exception(self): |
555 | + def test_check_declaration_slots_allow_connection_attrib_lists_supported(self): |
556 | """Test check_declaration - slots/allow-connection/attrib - lists match snap attribute list in decl list of lists""" |
557 | slots = {"iface-foo": {"interface": "foo", "attrib1": ["b", "a"]}} |
558 | self.set_test_snap_yaml("slots", slots) |
559 | @@ -3991,8 +4388,7 @@ slots: |
560 | try: |
561 | c.check_declaration() |
562 | except SnapDeclarationException: |
563 | - return |
564 | - raise Exception("base declaration should be invalid") # pragma: nocover |
565 | + raise Exception("base declaration should not be invalid") # pragma: nocover |
566 | |
567 | def test_check_declaration_slots_allow_connection_attrib_list_nomatch(self): |
568 | """Test check_declaration - slots/allow-connection/attrib - list nomatch snap attribute str not in decl list""" |
LGTM - thanks for the detailed test cases.