Merge lp:~julian-edwards/maas/dup-boot-source-selections-part2-bug-1360280 into lp:~maas-committers/maas/trunk
- dup-boot-source-selections-part2-bug-1360280
- Merge into trunk
| Status: | Merged |
|---|---|
| Approved by: | Julian Edwards |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 3367 |
| Proposed branch: | lp:~julian-edwards/maas/dup-boot-source-selections-part2-bug-1360280 |
| Merge into: | lp:~maas-committers/maas/trunk |
| Prerequisite: | lp:~julian-edwards/maas/dup-boot-source-selections-bug-1360280 |
| Diff against target: |
466 lines (+321/-38) 4 files modified
src/maasserver/api/tests/test_boot_source_selections.py (+24/-13) src/maasserver/forms.py (+56/-0) src/maasserver/testing/factory.py (+6/-0) src/maasserver/tests/test_forms_bootsourceselection.py (+235/-25) |
| To merge this branch: | bzr merge lp:~julian-edwards/maas/dup-boot-source-selections-part2-bug-1360280 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Graham Binns (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Validate BootSourceSelection changes against the available selection that lives in BootSourceCache. Previously, bogus entries could be added.
Description of the change
Whoever reviews this, please double check that this won't affect custom images. I don't think it will, but ...
| Julian Edwards (julian-edwards) wrote : | # |
On Wednesday 12 Nov 2014 07:37:38 you wrote:
> I can *hear* jtv, here and elsewhere, saying "What is this obsession with
> creating 3 _things_?"
Arf :)
> Also, I don't actually understand from the test why you're creating three
> mostly identical BootSourceCaches, here and elsewhere. What does it
> actually achieve? Wouldn't one do just as well?
You figured this out....
> > + boot_caches.
> > + boot_source, arch=factory.
> > + os=os, release=release))
> > +
> > + params = {
> > + 'os': os,
> > + 'release': release,
> > + 'arches': [boot_caches[
>
> Ah, I get it now; you're combining the valid values for the field you're
> testing on from some but not all of the BootSourceCaches you've created.
>
> Consider moving the for loop into a helper. Also, add a comment (or a
> docstring on the helper so that you're not having to C&P comments all over
> the shop) explaining why you're creating multiple BootSourceCaches and what
> they'll be used for.
I've been staring at the code for so long that it didn't look that bad any
more. I guess I'll try for a helper - I ran out of brain juice on this one.
Thanks for reviewing!
| MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~julian-edwards/maas/dup-boot-source-selections-part2-bug-1360280 into lp:maas failed. Below is the output from the failed tests.
Ign http://
Get:1 http://
Ign http://
Get:2 http://
Ign http://
Hit http://
Get:3 http://
Hit http://
Get:4 http://
Get:5 http://
Hit http://
Get:6 http://
Get:7 http://
Get:8 http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Get:9 http://
Get:10 http://
Get:11 http://
Get:12 http://
Hit http://
Hit http://
Ign http://
Ign http://
Fetched 1,200 kB in 2s (411 kB/s)
Reading package lists...
sudo DEBIAN_
--
| Julian Edwards (julian-edwards) wrote : | # |
Thanks for pushing me to write a helper Graham, it was needed in the API tests that had become broken because of the extra validation!
| MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~julian-edwards/maas/dup-boot-source-selections-part2-bug-1360280 into lp:maas failed. Below is the output from the failed tests.
Ign http://
Get:1 http://
Ign http://
Get:2 http://
Ign http://
Hit http://
Get:3 http://
Hit http://
Get:4 http://
Get:5 http://
Hit http://
Get:6 http://
Get:7 http://
Get:8 http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Get:9 http://
Hit http://
Hit http://
Get:10 http://
Get:11 http://
Get:12 http://
Hit http://
Hit http://
Ign http://
Ign http://
Fetched 1,200 kB in 2s (411 kB/s)
Reading package lists...
sudo DEBIAN_
--
| MAAS Lander (maas-lander) wrote : | # |
The attempt to merge lp:~julian-edwards/maas/dup-boot-source-selections-part2-bug-1360280 into lp:maas failed. Below is the output from the failed tests.
Ign http://
Get:1 http://
Get:2 http://
Ign http://
Ign http://
Hit http://
Get:3 http://
Hit http://
Get:4 http://
Get:5 http://
Hit http://
Get:6 http://
Get:7 http://
Get:8 http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Hit http://
Get:9 http://
Get:10 http://
Get:11 http://
Get:12 http://
Hit http://
Hit http://
Ign http://
Ign http://
Fetched 1,200 kB in 2s (418 kB/s)
Reading package lists...
sudo DEBIAN_
--
| Julian Edwards (julian-edwards) wrote : | # |
It would help if I remembered to push my changes up.
Preview Diff
| 1 | === modified file 'src/maasserver/api/tests/test_boot_source_selections.py' | |||
| 2 | --- src/maasserver/api/tests/test_boot_source_selections.py 2014-10-23 15:26:37 +0000 | |||
| 3 | +++ src/maasserver/api/tests/test_boot_source_selections.py 2014-11-13 03:13:37 +0000 | |||
| 4 | @@ -117,17 +117,20 @@ | |||
| 5 | 117 | boot_source_selection = factory.make_BootSourceSelection() | 117 | boot_source_selection = factory.make_BootSourceSelection() |
| 6 | 118 | new_os = factory.make_name('os') | 118 | new_os = factory.make_name('os') |
| 7 | 119 | new_release = factory.make_name('release') | 119 | new_release = factory.make_name('release') |
| 8 | 120 | boot_source_caches = factory.make_many_BootSourceCaches( | ||
| 9 | 121 | 2, boot_source=boot_source_selection.boot_source, os=new_os, | ||
| 10 | 122 | release=new_release) | ||
| 11 | 120 | new_values = { | 123 | new_values = { |
| 12 | 121 | 'os': new_os, | 124 | 'os': new_os, |
| 13 | 122 | 'release': new_release, | 125 | 'release': new_release, |
| 15 | 123 | 'arches': [factory.make_name('arch'), factory.make_name('arch')], | 126 | 'arches': [boot_source_caches[0].arch, boot_source_caches[1].arch], |
| 16 | 124 | 'subarches': [ | 127 | 'subarches': [ |
| 19 | 125 | factory.make_name('subarch'), factory.make_name('subarch')], | 128 | boot_source_caches[0].subarch, boot_source_caches[1].subarch], |
| 20 | 126 | 'labels': [factory.make_name('label')], | 129 | 'labels': [boot_source_caches[0].label], |
| 21 | 127 | } | 130 | } |
| 22 | 128 | response = self.client_put( | 131 | response = self.client_put( |
| 23 | 129 | get_boot_source_selection_uri(boot_source_selection), new_values) | 132 | get_boot_source_selection_uri(boot_source_selection), new_values) |
| 25 | 130 | self.assertEqual(httplib.OK, response.status_code) | 133 | self.assertEqual(httplib.OK, response.status_code, response.content) |
| 26 | 131 | boot_source_selection = reload_object(boot_source_selection) | 134 | boot_source_selection = reload_object(boot_source_selection) |
| 27 | 132 | self.assertAttributes(boot_source_selection, new_values) | 135 | self.assertAttributes(boot_source_selection, new_values) |
| 28 | 133 | 136 | ||
| 29 | @@ -216,12 +219,15 @@ | |||
| 30 | 216 | self.become_admin() | 219 | self.become_admin() |
| 31 | 217 | boot_source_selection = factory.make_BootSourceSelection() | 220 | boot_source_selection = factory.make_BootSourceSelection() |
| 32 | 218 | new_release = factory.make_name('release') | 221 | new_release = factory.make_name('release') |
| 33 | 222 | boot_source_caches = factory.make_many_BootSourceCaches( | ||
| 34 | 223 | 2, boot_source=boot_source_selection.boot_source, | ||
| 35 | 224 | release=new_release) | ||
| 36 | 219 | new_values = { | 225 | new_values = { |
| 37 | 220 | 'release': new_release, | 226 | 'release': new_release, |
| 39 | 221 | 'arches': [factory.make_name('arch'), factory.make_name('arch')], | 227 | 'arches': [boot_source_caches[0].arch, boot_source_caches[1].arch], |
| 40 | 222 | 'subarches': [ | 228 | 'subarches': [ |
| 43 | 223 | factory.make_name('subarch'), factory.make_name('subarch')], | 229 | boot_source_caches[0].subarch, boot_source_caches[1].subarch], |
| 44 | 224 | 'labels': [factory.make_name('label')], | 230 | 'labels': [boot_source_caches[0].label], |
| 45 | 225 | } | 231 | } |
| 46 | 226 | response = self.client_put( | 232 | response = self.client_put( |
| 47 | 227 | get_boot_source_selection_backward_uri( | 233 | get_boot_source_selection_backward_uri( |
| 48 | @@ -279,12 +285,15 @@ | |||
| 49 | 279 | self.become_admin() | 285 | self.become_admin() |
| 50 | 280 | boot_source = factory.make_BootSource() | 286 | boot_source = factory.make_BootSource() |
| 51 | 281 | new_release = factory.make_name('release') | 287 | new_release = factory.make_name('release') |
| 52 | 288 | boot_source_caches = factory.make_many_BootSourceCaches( | ||
| 53 | 289 | 2, boot_source=boot_source, | ||
| 54 | 290 | release=new_release) | ||
| 55 | 282 | params = { | 291 | params = { |
| 56 | 283 | 'release': new_release, | 292 | 'release': new_release, |
| 58 | 284 | 'arches': [factory.make_name('arch'), factory.make_name('arch')], | 293 | 'arches': [boot_source_caches[0].arch, boot_source_caches[1].arch], |
| 59 | 285 | 'subarches': [ | 294 | 'subarches': [ |
| 62 | 286 | factory.make_name('subarch'), factory.make_name('subarch')], | 295 | boot_source_caches[0].subarch, boot_source_caches[1].subarch], |
| 63 | 287 | 'labels': [factory.make_name('label')], | 296 | 'labels': [boot_source_caches[0].label], |
| 64 | 288 | } | 297 | } |
| 65 | 289 | response = self.client.post( | 298 | response = self.client.post( |
| 66 | 290 | reverse( | 299 | reverse( |
| 67 | @@ -373,12 +382,14 @@ | |||
| 68 | 373 | self.become_admin() | 382 | self.become_admin() |
| 69 | 374 | boot_source = factory.make_BootSource() | 383 | boot_source = factory.make_BootSource() |
| 70 | 375 | new_release = factory.make_name('release') | 384 | new_release = factory.make_name('release') |
| 71 | 385 | boot_source_caches = factory.make_many_BootSourceCaches( | ||
| 72 | 386 | 2, boot_source=boot_source, release=new_release) | ||
| 73 | 376 | params = { | 387 | params = { |
| 74 | 377 | 'release': new_release, | 388 | 'release': new_release, |
| 76 | 378 | 'arches': [factory.make_name('arch'), factory.make_name('arch')], | 389 | 'arches': [boot_source_caches[0].arch, boot_source_caches[1].arch], |
| 77 | 379 | 'subarches': [ | 390 | 'subarches': [ |
| 80 | 380 | factory.make_name('subarch'), factory.make_name('subarch')], | 391 | boot_source_caches[0].subarch, boot_source_caches[1].subarch], |
| 81 | 381 | 'labels': [factory.make_name('label')], | 392 | 'labels': [boot_source_caches[0].label], |
| 82 | 382 | } | 393 | } |
| 83 | 383 | response = self.client.post(self.get_uri(boot_source), params) | 394 | response = self.client.post(self.get_uri(boot_source), params) |
| 84 | 384 | self.assertEqual(httplib.OK, response.status_code) | 395 | self.assertEqual(httplib.OK, response.status_code) |
| 85 | 385 | 396 | ||
| 86 | === modified file 'src/maasserver/forms.py' | |||
| 87 | --- src/maasserver/forms.py 2014-11-07 15:33:47 +0000 | |||
| 88 | +++ src/maasserver/forms.py 2014-11-13 03:13:37 +0000 | |||
| 89 | @@ -109,6 +109,7 @@ | |||
| 90 | 109 | BootResourceFile, | 109 | BootResourceFile, |
| 91 | 110 | BootResourceSet, | 110 | BootResourceSet, |
| 92 | 111 | BootSource, | 111 | BootSource, |
| 93 | 112 | BootSourceCache, | ||
| 94 | 112 | BootSourceSelection, | 113 | BootSourceSelection, |
| 95 | 113 | Config, | 114 | Config, |
| 96 | 114 | DownloadProgress, | 115 | DownloadProgress, |
| 97 | @@ -2354,6 +2355,61 @@ | |||
| 98 | 2354 | else: | 2355 | else: |
| 99 | 2355 | self.boot_source = boot_source | 2356 | self.boot_source = boot_source |
| 100 | 2356 | 2357 | ||
| 101 | 2358 | def clean(self): | ||
| 102 | 2359 | cleaned_data = super(BootSourceSelectionForm, self).clean() | ||
| 103 | 2360 | |||
| 104 | 2361 | # Don't filter on OS if not provided. This is to maintain | ||
| 105 | 2362 | # backwards compatibility for when OS didn't exist in the API. | ||
| 106 | 2363 | if cleaned_data['os']: | ||
| 107 | 2364 | cache = BootSourceCache.objects.filter( | ||
| 108 | 2365 | boot_source=self.boot_source, os=cleaned_data['os'], | ||
| 109 | 2366 | release=cleaned_data['release']) | ||
| 110 | 2367 | else: | ||
| 111 | 2368 | cache = BootSourceCache.objects.filter( | ||
| 112 | 2369 | boot_source=self.boot_source, release=cleaned_data['release']) | ||
| 113 | 2370 | |||
| 114 | 2371 | if not cache.exists(): | ||
| 115 | 2372 | set_form_error( | ||
| 116 | 2373 | self, "os", | ||
| 117 | 2374 | "OS %s with release %s has no available images for download" % | ||
| 118 | 2375 | (cleaned_data['os'], cleaned_data['release'])) | ||
| 119 | 2376 | return cleaned_data | ||
| 120 | 2377 | |||
| 121 | 2378 | values = cache.values_list("arch", "subarch", "label") | ||
| 122 | 2379 | arches, subarches, labels = zip(*values) | ||
| 123 | 2380 | |||
| 124 | 2381 | # Validate architectures. | ||
| 125 | 2382 | required_arches_set = set(arch for arch in cleaned_data['arches']) | ||
| 126 | 2383 | wildcard_arches = '*' in required_arches_set | ||
| 127 | 2384 | if not wildcard_arches and not required_arches_set <= set(arches): | ||
| 128 | 2385 | set_form_error( | ||
| 129 | 2386 | self, "arches", | ||
| 130 | 2387 | "No available images to download for %s" % | ||
| 131 | 2388 | cleaned_data['arches']) | ||
| 132 | 2389 | |||
| 133 | 2390 | # Validate subarchitectures. | ||
| 134 | 2391 | required_subarches_set = set(sa for sa in cleaned_data['subarches']) | ||
| 135 | 2392 | wildcard_subarches = '*' in required_subarches_set | ||
| 136 | 2393 | if ( | ||
| 137 | 2394 | not wildcard_subarches and | ||
| 138 | 2395 | not required_subarches_set <= set(subarches) | ||
| 139 | 2396 | ): | ||
| 140 | 2397 | set_form_error( | ||
| 141 | 2398 | self, "subarches", | ||
| 142 | 2399 | "No available images to download for %s" % | ||
| 143 | 2400 | cleaned_data['subarches']) | ||
| 144 | 2401 | |||
| 145 | 2402 | # Validate labels. | ||
| 146 | 2403 | required_labels_set = set(label for label in cleaned_data['labels']) | ||
| 147 | 2404 | wildcard_labels = '*' in required_labels_set | ||
| 148 | 2405 | if not wildcard_labels and not required_labels_set <= set(labels): | ||
| 149 | 2406 | set_form_error( | ||
| 150 | 2407 | self, "labels", | ||
| 151 | 2408 | "No available images to download for %s" % | ||
| 152 | 2409 | cleaned_data['labels']) | ||
| 153 | 2410 | |||
| 154 | 2411 | return cleaned_data | ||
| 155 | 2412 | |||
| 156 | 2357 | def save(self, *args, **kwargs): | 2413 | def save(self, *args, **kwargs): |
| 157 | 2358 | boot_source_selection = super( | 2414 | boot_source_selection = super( |
| 158 | 2359 | BootSourceSelectionForm, self).save(commit=False) | 2415 | BootSourceSelectionForm, self).save(commit=False) |
| 159 | 2360 | 2416 | ||
| 160 | === modified file 'src/maasserver/testing/factory.py' | |||
| 161 | --- src/maasserver/testing/factory.py 2014-11-07 16:54:51 +0000 | |||
| 162 | +++ src/maasserver/testing/factory.py 2014-11-13 03:13:37 +0000 | |||
| 163 | @@ -891,6 +891,12 @@ | |||
| 164 | 891 | boot_source=boot_source, os=os, arch=arch, | 891 | boot_source=boot_source, os=os, arch=arch, |
| 165 | 892 | subarch=subarch, release=release, label=label) | 892 | subarch=subarch, release=release, label=label) |
| 166 | 893 | 893 | ||
| 167 | 894 | def make_many_BootSourceCaches(self, number, **kwargs): | ||
| 168 | 895 | caches = list() | ||
| 169 | 896 | for _ in range(number): | ||
| 170 | 897 | caches.append(self.make_BootSourceCache(**kwargs)) | ||
| 171 | 898 | return caches | ||
| 172 | 899 | |||
| 173 | 894 | def make_BootSourceSelection(self, boot_source=None, os=None, | 900 | def make_BootSourceSelection(self, boot_source=None, os=None, |
| 174 | 895 | release=None, arches=None, subarches=None, | 901 | release=None, arches=None, subarches=None, |
| 175 | 896 | labels=None): | 902 | labels=None): |
| 176 | 897 | 903 | ||
| 177 | === modified file 'src/maasserver/tests/test_forms_bootsourceselection.py' | |||
| 178 | --- src/maasserver/tests/test_forms_bootsourceselection.py 2014-11-11 05:15:23 +0000 | |||
| 179 | +++ src/maasserver/tests/test_forms_bootsourceselection.py 2014-11-13 03:13:37 +0000 | |||
| 180 | @@ -24,16 +24,47 @@ | |||
| 181 | 24 | class TestBootSourceSelectionForm(MAASServerTestCase): | 24 | class TestBootSourceSelectionForm(MAASServerTestCase): |
| 182 | 25 | """Tests for `BootSourceSelectionForm`.""" | 25 | """Tests for `BootSourceSelectionForm`.""" |
| 183 | 26 | 26 | ||
| 184 | 27 | def make_valid_source_selection_params(self, boot_source=None): | ||
| 185 | 28 | # Helper that creates a valid BootSourceCache and parameters for | ||
| 186 | 29 | # a BootSourceSelectionForm that will validate against the | ||
| 187 | 30 | # cache. | ||
| 188 | 31 | if boot_source is None: | ||
| 189 | 32 | boot_source = factory.make_BootSource() | ||
| 190 | 33 | arch = factory.make_name('arch') | ||
| 191 | 34 | arch2 = factory.make_name('arch') | ||
| 192 | 35 | subarch = factory.make_name('subarch') | ||
| 193 | 36 | subarch2 = factory.make_name('subarch') | ||
| 194 | 37 | label = factory.make_name('label') | ||
| 195 | 38 | label2 = factory.make_name('label') | ||
| 196 | 39 | params = { | ||
| 197 | 40 | 'os': factory.make_name('os'), | ||
| 198 | 41 | 'release': factory.make_name('release'), | ||
| 199 | 42 | 'arches': [arch, arch2], | ||
| 200 | 43 | 'subarches': [subarch, subarch2], | ||
| 201 | 44 | 'labels': [label, label2], | ||
| 202 | 45 | } | ||
| 203 | 46 | factory.make_BootSourceCache( | ||
| 204 | 47 | boot_source=boot_source, | ||
| 205 | 48 | os=params['os'], | ||
| 206 | 49 | release=params['release'], | ||
| 207 | 50 | arch=arch, | ||
| 208 | 51 | subarch=subarch, | ||
| 209 | 52 | label=label, | ||
| 210 | 53 | ) | ||
| 211 | 54 | factory.make_BootSourceCache( | ||
| 212 | 55 | boot_source=boot_source, | ||
| 213 | 56 | os=params['os'], | ||
| 214 | 57 | release=params['release'], | ||
| 215 | 58 | arch=arch2, | ||
| 216 | 59 | subarch=subarch2, | ||
| 217 | 60 | label=label2, | ||
| 218 | 61 | ) | ||
| 219 | 62 | return params | ||
| 220 | 63 | |||
| 221 | 27 | def test_edits_boot_source_selection_object(self): | 64 | def test_edits_boot_source_selection_object(self): |
| 222 | 28 | boot_source_selection = factory.make_BootSourceSelection() | 65 | boot_source_selection = factory.make_BootSourceSelection() |
| 231 | 29 | params = { | 66 | boot_source = boot_source_selection.boot_source |
| 232 | 30 | 'os': factory.make_name('os'), | 67 | params = self.make_valid_source_selection_params(boot_source) |
| 225 | 31 | 'release': factory.make_name('release'), | ||
| 226 | 32 | 'arches': [factory.make_name('arch'), factory.make_name('arch')], | ||
| 227 | 33 | 'subarches': [ | ||
| 228 | 34 | factory.make_name('subarch'), factory.make_name('subarch')], | ||
| 229 | 35 | 'labels': [factory.make_name('label'), factory.make_name('label')], | ||
| 230 | 36 | } | ||
| 233 | 37 | form = BootSourceSelectionForm( | 68 | form = BootSourceSelectionForm( |
| 234 | 38 | instance=boot_source_selection, data=params) | 69 | instance=boot_source_selection, data=params) |
| 235 | 39 | self.assertTrue(form.is_valid(), form._errors) | 70 | self.assertTrue(form.is_valid(), form._errors) |
| 236 | @@ -43,14 +74,7 @@ | |||
| 237 | 43 | 74 | ||
| 238 | 44 | def test_creates_boot_source_selection_object(self): | 75 | def test_creates_boot_source_selection_object(self): |
| 239 | 45 | boot_source = factory.make_BootSource() | 76 | boot_source = factory.make_BootSource() |
| 248 | 46 | params = { | 77 | params = self.make_valid_source_selection_params(boot_source) |
| 241 | 47 | 'os': factory.make_name('os'), | ||
| 242 | 48 | 'release': factory.make_name('release'), | ||
| 243 | 49 | 'arches': [factory.make_name('arch'), factory.make_name('arch')], | ||
| 244 | 50 | 'subarches': [ | ||
| 245 | 51 | factory.make_name('subarch'), factory.make_name('subarch')], | ||
| 246 | 52 | 'labels': [factory.make_name('label'), factory.make_name('label')], | ||
| 247 | 53 | } | ||
| 249 | 54 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | 78 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) |
| 250 | 55 | self.assertTrue(form.is_valid(), form._errors) | 79 | self.assertTrue(form.is_valid(), form._errors) |
| 251 | 56 | boot_source_selection = form.save() | 80 | boot_source_selection = form.save() |
| 252 | @@ -58,15 +82,9 @@ | |||
| 253 | 58 | 82 | ||
| 254 | 59 | def test_cannot_create_duplicate_entry(self): | 83 | def test_cannot_create_duplicate_entry(self): |
| 255 | 60 | boot_source = factory.make_BootSource() | 84 | boot_source = factory.make_BootSource() |
| 265 | 61 | params = { | 85 | params = self.make_valid_source_selection_params(boot_source) |
| 266 | 62 | 'os': factory.make_name('os'), | 86 | form = BootSourceSelectionForm( |
| 267 | 63 | 'release': factory.make_name('release'), | 87 | boot_source=boot_source, data=params) |
| 259 | 64 | 'arches': [factory.make_name('arch'), factory.make_name('arch')], | ||
| 260 | 65 | 'subarches': [ | ||
| 261 | 66 | factory.make_name('subarch'), factory.make_name('subarch')], | ||
| 262 | 67 | 'labels': [factory.make_name('label'), factory.make_name('label')], | ||
| 263 | 68 | } | ||
| 264 | 69 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 268 | 70 | self.assertTrue(form.is_valid(), form._errors) | 88 | self.assertTrue(form.is_valid(), form._errors) |
| 269 | 71 | form.save() | 89 | form.save() |
| 270 | 72 | 90 | ||
| 271 | @@ -79,3 +97,195 @@ | |||
| 272 | 79 | form = BootSourceSelectionForm( | 97 | form = BootSourceSelectionForm( |
| 273 | 80 | boot_source=boot_source, data=dup_params) | 98 | boot_source=boot_source, data=dup_params) |
| 274 | 81 | self.assertRaises(ValidationError, form.save) | 99 | self.assertRaises(ValidationError, form.save) |
| 275 | 100 | |||
| 276 | 101 | def test_validates_if_boot_source_cache_has_same_os_and_release(self): | ||
| 277 | 102 | boot_source = factory.make_BootSource() | ||
| 278 | 103 | boot_cache = factory.make_BootSourceCache(boot_source) | ||
| 279 | 104 | |||
| 280 | 105 | params = { | ||
| 281 | 106 | 'os': boot_cache.os, | ||
| 282 | 107 | 'release': boot_cache.release, | ||
| 283 | 108 | } | ||
| 284 | 109 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 285 | 110 | self.assertTrue(form.is_valid(), form._errors) | ||
| 286 | 111 | |||
| 287 | 112 | def test_rejects_if_boot_source_cache_has_different_os(self): | ||
| 288 | 113 | boot_source = factory.make_BootSource() | ||
| 289 | 114 | boot_cache = factory.make_BootSourceCache(boot_source) | ||
| 290 | 115 | |||
| 291 | 116 | params = { | ||
| 292 | 117 | 'os': factory.make_name('os'), | ||
| 293 | 118 | 'release': boot_cache.release, | ||
| 294 | 119 | } | ||
| 295 | 120 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 296 | 121 | self.assertFalse(form.is_valid()) | ||
| 297 | 122 | self.assertEqual( | ||
| 298 | 123 | { | ||
| 299 | 124 | "os": [ | ||
| 300 | 125 | "OS %s with release %s has no available images " | ||
| 301 | 126 | "for download" % (params['os'], boot_cache.release) | ||
| 302 | 127 | ] | ||
| 303 | 128 | }, | ||
| 304 | 129 | form._errors) | ||
| 305 | 130 | |||
| 306 | 131 | def test_rejects_if_boot_source_cache_has_different_release(self): | ||
| 307 | 132 | boot_source = factory.make_BootSource() | ||
| 308 | 133 | boot_cache = factory.make_BootSourceCache(boot_source) | ||
| 309 | 134 | |||
| 310 | 135 | params = { | ||
| 311 | 136 | 'os': boot_cache.os, | ||
| 312 | 137 | 'release': factory.make_name('release'), | ||
| 313 | 138 | } | ||
| 314 | 139 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 315 | 140 | self.assertFalse(form.is_valid()) | ||
| 316 | 141 | self.assertEqual( | ||
| 317 | 142 | { | ||
| 318 | 143 | "os": [ | ||
| 319 | 144 | "OS %s with release %s has no available images " | ||
| 320 | 145 | "for download" % (boot_cache.os, params['release']) | ||
| 321 | 146 | ] | ||
| 322 | 147 | }, | ||
| 323 | 148 | form._errors) | ||
| 324 | 149 | |||
| 325 | 150 | def make_some_caches(self, boot_source, os, release): | ||
| 326 | 151 | # Make a few BootSourceCache records that the following tests can use | ||
| 327 | 152 | # to validate against when using BootSourceSelectionForm. | ||
| 328 | 153 | return factory.make_many_BootSourceCaches( | ||
| 329 | 154 | 3, boot_source=boot_source, os=os, release=release) | ||
| 330 | 155 | |||
| 331 | 156 | def test_validates_if_boot_source_cache_has_arch(self): | ||
| 332 | 157 | boot_source = factory.make_BootSource() | ||
| 333 | 158 | os = factory.make_name('os') | ||
| 334 | 159 | release = factory.make_name('release') | ||
| 335 | 160 | boot_caches = self.make_some_caches(boot_source, os, release) | ||
| 336 | 161 | |||
| 337 | 162 | # Request arches that are in two of the cache records. | ||
| 338 | 163 | params = { | ||
| 339 | 164 | 'os': os, | ||
| 340 | 165 | 'release': release, | ||
| 341 | 166 | 'arches': [boot_caches[0].arch, boot_caches[2].arch], | ||
| 342 | 167 | } | ||
| 343 | 168 | |||
| 344 | 169 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 345 | 170 | self.assertTrue(form.is_valid(), form._errors) | ||
| 346 | 171 | |||
| 347 | 172 | def test_rejects_if_boot_source_cache_does_not_have_arch(self): | ||
| 348 | 173 | boot_source = factory.make_BootSource() | ||
| 349 | 174 | os = factory.make_name('os') | ||
| 350 | 175 | release = factory.make_name('release') | ||
| 351 | 176 | factory.make_BootSourceCache( | ||
| 352 | 177 | boot_source, os=os, release=release) | ||
| 353 | 178 | |||
| 354 | 179 | params = { | ||
| 355 | 180 | 'os': os, | ||
| 356 | 181 | 'release': release, | ||
| 357 | 182 | 'arches': [factory.make_name('arch')], | ||
| 358 | 183 | } | ||
| 359 | 184 | |||
| 360 | 185 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 361 | 186 | self.assertFalse(form.is_valid()) | ||
| 362 | 187 | self.assertEqual( | ||
| 363 | 188 | { | ||
| 364 | 189 | "arches": [ | ||
| 365 | 190 | "No available images to download for %s" % | ||
| 366 | 191 | params['arches'] | ||
| 367 | 192 | ] | ||
| 368 | 193 | }, | ||
| 369 | 194 | form._errors) | ||
| 370 | 195 | |||
| 371 | 196 | def test_validates_if_boot_source_cache_has_subarch(self): | ||
| 372 | 197 | boot_source = factory.make_BootSource() | ||
| 373 | 198 | os = factory.make_name('os') | ||
| 374 | 199 | release = factory.make_name('release') | ||
| 375 | 200 | boot_caches = self.make_some_caches(boot_source, os, release) | ||
| 376 | 201 | |||
| 377 | 202 | # Request subarches that are in two of the cache records. | ||
| 378 | 203 | params = { | ||
| 379 | 204 | 'os': os, | ||
| 380 | 205 | 'release': release, | ||
| 381 | 206 | 'subarches': [boot_caches[0].subarch, boot_caches[2].subarch], | ||
| 382 | 207 | } | ||
| 383 | 208 | |||
| 384 | 209 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 385 | 210 | self.assertTrue(form.is_valid(), form._errors) | ||
| 386 | 211 | |||
| 387 | 212 | def test_rejects_if_boot_source_cache_does_not_have_subarch(self): | ||
| 388 | 213 | boot_source = factory.make_BootSource() | ||
| 389 | 214 | os = factory.make_name('os') | ||
| 390 | 215 | release = factory.make_name('release') | ||
| 391 | 216 | factory.make_BootSourceCache( | ||
| 392 | 217 | boot_source, os=os, release=release) | ||
| 393 | 218 | |||
| 394 | 219 | params = { | ||
| 395 | 220 | 'os': os, | ||
| 396 | 221 | 'release': release, | ||
| 397 | 222 | 'subarches': [factory.make_name('subarch')], | ||
| 398 | 223 | } | ||
| 399 | 224 | |||
| 400 | 225 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 401 | 226 | self.assertFalse(form.is_valid()) | ||
| 402 | 227 | self.assertEqual( | ||
| 403 | 228 | { | ||
| 404 | 229 | "subarches": [ | ||
| 405 | 230 | "No available images to download for %s" % | ||
| 406 | 231 | params['subarches'] | ||
| 407 | 232 | ] | ||
| 408 | 233 | }, | ||
| 409 | 234 | form._errors) | ||
| 410 | 235 | |||
| 411 | 236 | def test_validates_if_boot_source_cache_has_label(self): | ||
| 412 | 237 | boot_source = factory.make_BootSource() | ||
| 413 | 238 | os = factory.make_name('os') | ||
| 414 | 239 | release = factory.make_name('release') | ||
| 415 | 240 | boot_caches = self.make_some_caches(boot_source, os, release) | ||
| 416 | 241 | |||
| 417 | 242 | # Request labels that are in two of the cache records. | ||
| 418 | 243 | params = { | ||
| 419 | 244 | 'os': os, | ||
| 420 | 245 | 'release': release, | ||
| 421 | 246 | 'labels': [boot_caches[0].label, boot_caches[2].label], | ||
| 422 | 247 | } | ||
| 423 | 248 | |||
| 424 | 249 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 425 | 250 | self.assertTrue(form.is_valid(), form._errors) | ||
| 426 | 251 | |||
| 427 | 252 | def test_rejects_if_boot_source_cache_does_not_have_label(self): | ||
| 428 | 253 | boot_source = factory.make_BootSource() | ||
| 429 | 254 | os = factory.make_name('os') | ||
| 430 | 255 | release = factory.make_name('release') | ||
| 431 | 256 | factory.make_BootSourceCache( | ||
| 432 | 257 | boot_source, os=os, release=release) | ||
| 433 | 258 | |||
| 434 | 259 | params = { | ||
| 435 | 260 | 'os': os, | ||
| 436 | 261 | 'release': release, | ||
| 437 | 262 | 'labels': [factory.make_name('label')], | ||
| 438 | 263 | } | ||
| 439 | 264 | |||
| 440 | 265 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 441 | 266 | self.assertFalse(form.is_valid()) | ||
| 442 | 267 | self.assertEqual( | ||
| 443 | 268 | { | ||
| 444 | 269 | "labels": [ | ||
| 445 | 270 | "No available images to download for %s" % | ||
| 446 | 271 | params['labels'] | ||
| 447 | 272 | ] | ||
| 448 | 273 | }, | ||
| 449 | 274 | form._errors) | ||
| 450 | 275 | |||
| 451 | 276 | def test_star_values_in_request_validate_against_any_cache(self): | ||
| 452 | 277 | boot_source = factory.make_BootSource() | ||
| 453 | 278 | os = factory.make_name('os') | ||
| 454 | 279 | release = factory.make_name('release') | ||
| 455 | 280 | factory.make_BootSourceCache( | ||
| 456 | 281 | boot_source, os=os, release=release) | ||
| 457 | 282 | params = { | ||
| 458 | 283 | 'os': os, | ||
| 459 | 284 | 'release': release, | ||
| 460 | 285 | 'arches': ['*'], | ||
| 461 | 286 | 'subarches': ['*'], | ||
| 462 | 287 | 'labels': ['*'], | ||
| 463 | 288 | } | ||
| 464 | 289 | |||
| 465 | 290 | form = BootSourceSelectionForm(boot_source=boot_source, data=params) | ||
| 466 | 291 | self.assertTrue(form.is_valid(), form._errors) | ||
Looks good. There's a don't-make-me-think nit inline, but otherwise this is fine.