Merge ~laney/simplestreams:laney/indexes-sort-products into simplestreams:master

Proposed by Iain Lane
Status: Merged
Approved by: Paride Legovini
Approved revision: 9f1df667db547a003f1469c019b676849ae9fa54
Merge reported by: Server Team CI bot
Merged at revision: not available
Proposed branch: ~laney/simplestreams:laney/indexes-sort-products
Merge into: simplestreams:master
Diff against target: 38 lines (+15/-1)
2 files modified
simplestreams/generate_simplestreams.py (+1/-1)
tests/unittests/test_generate_simplestreams.py (+14/-0)
Reviewer Review Type Date Requested Status
Server Team CI bot continuous-integration Approve
Paride Legovini Approve
Review via email: mp+430411@code.launchpad.net

Commit message

generate_index: Output product list in sorted order

This can help when consumers want to write code which
matches on the expected output generated by simplestreams.

Description of the change

We currently have a test in ubuntu-cdimage failing like this:

```
======================================================================
FAIL: test_release_tree (cdimage.tests.test_simplestreams.TestSimpleStreamsTree)
Check if we get a right simplestream for a release tree.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python3.10/unittest/mock.py", line 1369, in patched
    return func(*newargs, **newkeywargs)
  File "/home/laney/dev/ubuntu/ubuntu-cdimage/lib/cdimage/tests/test_simplestreams.py", line 367, in test_release_tree
    self.assertDictEqual(
AssertionError: {'for[271 chars]8.04:amd64', 'com.ubuntu.cdimage:kubuntu:deskt[1055 chars]MP'}} != {'for[271 chars]8.04:i386', 'com.ubuntu.cdimage:kubuntu:deskto[1055 chars]MP'}}
  {'format': 'index:1.0',
   'index': {'com.ubuntu.cdimage:kubuntu': {'datatype': 'image-downloads',
                                            'format': 'products:1.0',
                                            'path': 'streams/v1/com.ubuntu.cdimage:kubuntu.json',
                                            'products': ['com.ubuntu.cdimage:kubuntu:desktop:20.04:amd64',
+ 'com.ubuntu.cdimage:kubuntu:desktop:18.04:i386',
- 'com.ubuntu.cdimage:kubuntu:desktop:18.04:amd64',
+ 'com.ubuntu.cdimage:kubuntu:desktop:18.04:amd64'],
? +

- 'com.ubuntu.cdimage:kubuntu:desktop:18.04:i386'],
                                            'updated': 'TIMESTAMP'},
             'com.ubuntu.cdimage:ubuntu': {'datatype': 'image-downloads',
                                           'format': 'products:1.0',
                                           'path': 'streams/v1/com.ubuntu.cdimage:ubuntu.json',
                                           'products': ['com.ubuntu.cdimage:ubuntu:preinstalled-server:20.04:armhf+raspi',
+ 'com.ubuntu.cdimage:ubuntu:live-server:20.04:s390x',
+ 'com.ubuntu.cdimage:ubuntu:preinstalled-server:20.04:riscv64+unleashed',
+ 'com.ubuntu.cdimage:ubuntu:live-server:20.04:ppc64el',
+ 'com.ubuntu.cdimage:ubuntu:preinstalled-server:20.04:arm64+raspi',
                                                        'com.ubuntu.cdimage:ubuntu:preinstalled-server:20.04:riscv64+unmatched',
- 'com.ubuntu.cdimage:ubuntu:preinstalled-server:20.04:arm64+raspi',
- 'com.ubuntu.cdimage:ubuntu:preinstalled-server:20.04:riscv64+unleashed',
                                                        'com.ubuntu.cdimage:ubuntu:live-server:20.04:arm64',
- 'com.ubuntu.cdimage:ubuntu:live-server:20.04:ppc64el',
- 'com.ubuntu.cdimage:ubuntu:live-server:20.04:s390x',
? ^^ ^^ ---

+ 'com.ubuntu.cdimage:ubuntu:preinstalled-server:18.04:arm64+raspi3',
? ++++++++ ^ + ^^ ++++++++ ++

                                                        'com.ubuntu.cdimage:ubuntu:server:18.04:s390x',
- 'com.ubuntu.cdimage:ubuntu:preinstalled-server:18.04:arm64+raspi3',
                                                        'com.ubuntu.cdimage:ubuntu:server:18.04:amd64',
- 'com.ubuntu.cdimage:ubuntu:server:18.04:arm64',
                                                        'com.ubuntu.cdimage:ubuntu:server:18.04:ppc64el',
- 'com.ubuntu.cdimage:ubuntu:preinstalled-server:18.04:arm64+raspi4'],
? -

+ 'com.ubuntu.cdimage:ubuntu:preinstalled-server:18.04:arm64+raspi4',
+ 'com.ubuntu.cdimage:ubuntu:server:18.04:arm64'],
                                           'updated': 'TIMESTAMP'}},
   'updated': {'datatype': 'image-downloads', 'updated': 'TIMESTAMP'}} : SimpleStreams file index.json has unexpected contents.
```

It's trying to check that the cdimage code generates the correct simplestreams, but it's failing - not because the result is wrong - but because the order of the product list in the indexes is.

This MP will keep that list sorted, and then we can do that same on the cdimage side, and our comparisons will be able to be stable.

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

I can't access those CI runs by the way: "Either you have not been granted access to this resource or your entitlement has timed out. Please try again."

Revision history for this message
Paride Legovini (paride) wrote :

Hi Iain, LGTM, thanks!

On the CI output: that's not openly accessible, I'm sorry. It used to be (and as you can see the CI approval comment kind of assumes it is), but at some point all the Jenkins instances were made private.

review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

Commit message lints:
- Line #2 has 38 too many characters. Line starts with: "This can help when consumers"...

review: Needs Fixing
Revision history for this message
Paride Legovini (paride) wrote :

Rewrapped, approving again.

review: Approve
Revision history for this message
Server Team CI bot (server-team-bot) :
review: Approve (continuous-integration)
Revision history for this message
Iain Lane (laney) wrote :

Thanks paride! I can't merge so could you take care of that for me please?

I unwrapped the commit message in the field here for readability, whoops, sorry about that.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/simplestreams/generate_simplestreams.py b/simplestreams/generate_simplestreams.py
2index 5cb92b2..9b7b919 100644
3--- a/simplestreams/generate_simplestreams.py
4+++ b/simplestreams/generate_simplestreams.py
5@@ -72,7 +72,7 @@ def generate_index(trees, updated, namer):
6 not_copied_up = ['content_id']
7 for content_id, content in trees.items():
8 index['index'][content_id] = {
9- 'products': list(content['products'].keys()),
10+ 'products': sorted(list(content['products'].keys())),
11 'path': namer.get_content_path(content_id),
12 }
13 for k in util.stringitems(content):
14diff --git a/tests/unittests/test_generate_simplestreams.py b/tests/unittests/test_generate_simplestreams.py
15index a47a126..a24f541 100644
16--- a/tests/unittests/test_generate_simplestreams.py
17+++ b/tests/unittests/test_generate_simplestreams.py
18@@ -198,6 +198,20 @@ class TestGenerateIndex(TestCase):
19 }
20 }, index_json)
21
22+ def test_products_sorted(self):
23+ ''' The products list in the index should be sorted '''
24+ index_json = generate_index({
25+ 'foo': {'products': {'prodfooz': {}, 'prodfooa': {}}},
26+ }, self.updated, FakeNamer)
27+ self.assertEqual({
28+ 'format': 'index:1.0', 'updated': self.updated, 'index': {
29+ 'foo': {
30+ 'path': 'foo.json',
31+ 'products': ['prodfooa', 'prodfooz'],
32+ },
33+ }
34+ }, index_json)
35+
36
37 def load_stream_dir(stream_dir):
38 contents = {}

Subscribers

People subscribed via source and target branches

to all changes: