From 7b28a7f54e9f64dbccf7f54904a547f5d025defb Mon Sep 17 00:00:00 2001 From: Pieter De Gendt Date: Fri, 20 Sep 2024 14:34:23 +0200 Subject: [PATCH 1/3] manifest: Add submodule representation in as_dict Submodules were missing when calling --freeze or --resolve Signed-off-by: Pieter De Gendt --- src/west/manifest.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/west/manifest.py b/src/west/manifest.py index 57041eff..500f348e 100644 --- a/src/west/manifest.py +++ b/src/west/manifest.py @@ -882,6 +882,15 @@ def as_dict(self) -> Dict: _west_commands_maybe_delist(self.west_commands) if self.groups: ret['groups'] = self.groups + if isinstance(self.submodules, bool) and self.submodules: + ret['submodules'] = True + elif isinstance(self.submodules, list): + ret['submodules'] = [] + for s in self.submodules: + obj: Dict = {'path': s.path} + if s.name: + obj['name'] = s.name + ret['submodules'].append(obj) if self.userdata: ret['userdata'] = self.userdata @@ -1143,7 +1152,7 @@ def __init__(self, path: Optional[PathType] = None, # Pretending that this is a Project, even though it's not (#327) self.description: Optional[str] = None self.url: str = '' - self.submodules = False + self.submodules: SubmodulesType = False self.revision: str = 'HEAD' self.remote_name: str = '' self.clone_depth: Optional[int] = None From 6dcc76d61ac09c44169e4cf9825b19c29fdcdddb Mon Sep 17 00:00:00 2001 From: Pieter De Gendt Date: Fri, 20 Sep 2024 14:36:26 +0200 Subject: [PATCH 2/3] tests: Add tests for manifest --freeze and submodules Add a minimal testing to validate submodule output for manifest --freeze Signed-off-by: Pieter De Gendt --- tests/conftest.py | 2 ++ tests/test_project.py | 77 ++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5d0d79f4..5dabaf3e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -46,6 +46,7 @@ path: subdir/Kconfiglib groups: - Kconfiglib-group + submodules: true - name: tagged_repo revision: v1.0 - name: net-tools @@ -189,6 +190,7 @@ def repos_tmpdir(tmpdir, _session_repos): - name: Kconfiglib revision: zephyr path: subdir/Kconfiglib + submodules: true - name: tagged_repo revision: v1.0 - name: net-tools diff --git a/tests/test_project.py b/tests/test_project.py index 9ba7b6ad..c982c2dd 100644 --- a/tests/test_project.py +++ b/tests/test_project.py @@ -81,6 +81,11 @@ def _list_f(format): return ['list', '-f', format] +def _match_multiline_regex(expected, actual): + for eline_re, aline in zip(expected, actual): + assert re.match(eline_re, aline) is not None, (aline, eline_re) + + def test_workspace(west_update_tmpdir): # Basic test that west_update_tmpdir bootstrapped correctly. This # is a basic test of west init and west update. @@ -251,6 +256,7 @@ def test_manifest_freeze(west_update_tmpdir): '^ path: subdir/Kconfiglib$', '^ groups:$', '^ - Kconfiglib-group$', + '^ submodules: true$', '^ - name: tagged_repo$', '^ url: .*$', '^ revision: [a-f0-9]{40}$', @@ -262,10 +268,7 @@ def test_manifest_freeze(west_update_tmpdir): '^ west-commands: scripts/west-commands.yml$', '^ self:$', '^ path: zephyr$'] - - for eline_re, aline in zip(expected_res, actual): - assert re.match(eline_re, aline) is not None, (aline, eline_re) - + _match_multiline_regex(expected_res, actual) def test_compare(config_tmpdir, west_init_tmpdir): # 'west compare' with no projects cloned should still work, @@ -806,6 +809,26 @@ def test_update_submodules_list(repos_tmpdir): capture_stderr=True, capture_stdout=True) assert not (res.returncode or res.stdout.strip()) + # Test freeze output with submodules + # see test_manifest_freeze for details + actual = cmd('manifest --freeze', cwd=ws).splitlines() + expected_res = ['^manifest:$', + '^ projects:$', + '^ - name: zephyr$', + f'^ url: {re.escape(str(zephyr))}$', + '^ revision: [a-f0-9]{40}$', + '^ submodules:$', + '^ - path: tagged_repo$', + '^ - name: net-tools$', + f'^ url: {re.escape(str(net_tools))}$', + '^ revision: [a-f0-9]{40}$', + '^ submodules:$', + f'^ - path: {re.escape(str(kconfiglib_submodule))}$', + '^ name: Kconfiglib$', + '^ self:$', + '^ path: mp$'] + _match_multiline_regex(expected_res, actual) + def test_update_all_submodules(repos_tmpdir): # The west update command should not only update projects, # but also its submodules. Test verifies whether setting submodules @@ -899,6 +922,19 @@ def test_update_all_submodules(repos_tmpdir): capture_stderr=True, capture_stdout=True) assert not (res.returncode or res.stdout.strip()) + # Test freeze output with submodules + # see test_manifest_freeze for details + actual = cmd('manifest --freeze', cwd=ws).splitlines() + expected_res = ['^manifest:$', + '^ projects:$', + '^ - name: zephyr$', + f'^ url: {re.escape(str(zephyr))}$', + '^ revision: [a-f0-9]{40}$', + '^ submodules: true$', + '^ self:$', + '^ path: mp$'] + _match_multiline_regex(expected_res, actual) + def test_update_no_submodules(repos_tmpdir): # Test verifies whether setting submodules value to boolean False does not # result in updating project submodules. @@ -969,6 +1005,18 @@ def test_update_no_submodules(repos_tmpdir): capture_stderr=True, capture_stdout=True) assert (res.returncode or res.stdout.strip()) + # Test freeze output with submodules + # see test_manifest_freeze for details + actual = cmd('manifest --freeze', cwd=ws).splitlines() + expected_res = ['^manifest:$', + '^ projects:$', + '^ - name: zephyr$', + f'^ url: {re.escape(str(zephyr))}$', + '^ revision: [a-f0-9]{40}$', + '^ self:$', + '^ path: mp$'] + _match_multiline_regex(expected_res, actual) + def test_update_submodules_strategy(repos_tmpdir): # The west update command is able to update submodules using default # checkout strategy or rebase strategy, selected by adding -r argument @@ -1097,6 +1145,27 @@ def test_update_submodules_strategy(repos_tmpdir): assert net_tools_project.sha('HEAD', cwd=kconfiglib_dst_dir) \ == kconfiglib_new_sha + # Test freeze output with submodules + # see test_manifest_freeze for details + actual = cmd('manifest --freeze', cwd=ws).splitlines() + expected_res = ['^manifest:$', + '^ projects:$', + '^ - name: zephyr$', + f'^ url: {re.escape(str(zephyr))}$', + '^ revision: [a-f0-9]{40}$', + '^ submodules:$', + '^ - path: tagged_repo$', + '^ name: tagged_repo$', + '^ - name: net-tools$', + f'^ url: {re.escape(str(net_tools))}$', + '^ revision: [a-f0-9]{40}$', + '^ submodules:$', + '^ - path: Kconfiglib$', + '^ name: Kconfiglib$', + '^ self:$', + '^ path: mp$'] + _match_multiline_regex(expected_res, actual) + @pytest.mark.xfail def test_update_submodules_relpath(tmpdir): # Regression test for From 46d36fdf43774601ab9bc37369647b21a24a4c1d Mon Sep 17 00:00:00 2001 From: Pieter De Gendt Date: Mon, 23 Sep 2024 12:21:36 +0200 Subject: [PATCH 3/3] tests: Add manifest test for submodules Add a testcase to parse a manifest and validate the as_dict output. Add an invalid manifest file for missing required submodule properties. Signed-off-by: Pieter De Gendt --- tests/manifests/invalid_submodule_no_path.yml | 10 +++ tests/test_manifest.py | 68 +++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 tests/manifests/invalid_submodule_no_path.yml diff --git a/tests/manifests/invalid_submodule_no_path.yml b/tests/manifests/invalid_submodule_no_path.yml new file mode 100644 index 00000000..d0df5121 --- /dev/null +++ b/tests/manifests/invalid_submodule_no_path.yml @@ -0,0 +1,10 @@ +# Submodule entries require a path +manifest: + remotes: + - name: remote1 + url-base: https://example.com + projects: + - name: project1 + remote: remote1 + submodules: + - name: project2 diff --git a/tests/test_manifest.py b/tests/test_manifest.py index bae75bf0..9928af4e 100644 --- a/tests/test_manifest.py +++ b/tests/test_manifest.py @@ -3176,6 +3176,74 @@ def setup_project(name, group_filter): assert hasattr(m, '_legacy_group_filter_warned') +def test_submodule_manifest(): + m = M('''\ + projects: + - name: project1 + url: url + - name: project2 + url: url + submodules: true + - name: project3 + url: url + submodules: + - path: path + - name: project4 + url: url + submodules: + - path: path + name: subproject1 + - name: project5 + url: url + submodules: + - path: path + name: subproject1 + - path: path + name: subproject2 + - name: project6 + url: url + submodules: false + ''').as_dict()['manifest'] + + mp = m['projects'][0] + assert 'submodules' not in mp + + mp = m['projects'][1] + assert 'submodules' in mp + assert isinstance(mp['submodules'], bool) + assert mp['submodules'] + + mp = m['projects'][2] + assert isinstance(mp['submodules'], list) + assert len(mp['submodules']) == 1 + assert 'path' in mp['submodules'][0] + assert mp['submodules'][0]['path'] == 'path' + assert 'name' not in mp['submodules'][0] + + mp = m['projects'][3] + assert isinstance(mp['submodules'], list) + assert len(mp['submodules']) == 1 + assert 'path' in mp['submodules'][0] + assert mp['submodules'][0]['path'] == 'path' + assert 'name' in mp['submodules'][0] + assert mp['submodules'][0]['name'] == 'subproject1' + + mp = m['projects'][4] + assert isinstance(mp['submodules'], list) + assert len(mp['submodules']) == 2 + assert 'path' in mp['submodules'][0] + assert mp['submodules'][0]['path'] == 'path' + assert 'name' in mp['submodules'][0] + assert mp['submodules'][0]['name'] == 'subproject1' + assert 'path' in mp['submodules'][1] + assert mp['submodules'][1]['path'] == 'path' + assert 'name' in mp['submodules'][1] + assert mp['submodules'][1]['name'] == 'subproject2' + + mp = m['projects'][5] + assert 'submodules' not in mp + + ######################################### # Various invalid manifests