Skip to content

Commit b1e2d93

Browse files
[nrf noup][ZAP] Extend unit tests to check zap-generate full
Make sure that west zap-generate full that was run with -y argument creates the same content as zap-generate --full. Signed-off-by: Arkadiusz Balys <arkadiusz.balys@nordicsemi.no>
1 parent 7fd4772 commit b1e2d93

6 files changed

Lines changed: 190 additions & 55 deletions

File tree

.github/workflows/west-zap.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ jobs:
3434
steps:
3535
- name: Checkout
3636
uses: actions/checkout@v5
37-
- name: Checkout submodules & Bootstrap
38-
uses: ./.github/actions/checkout-submodules-and-bootstrap
37+
- name: Checkout submodules
38+
uses: ./.github/actions/checkout-submodules
3939
with:
4040
platform: nrfconnect
4141
- name: Prepare environment

scripts/west/tests/zap_samples.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
# This file is used to generate the ZAP files for the samples.
2+
# Use this file as an argument to the west zap-generate command:
3+
#
4+
# west zap-generate -y zap_samples.yml
5+
#
6+
7+
# Base dir related to ZEPHYR_BASE
8+
- base_dir: ../modules/lib/matter/test_dir
9+
10+
- name: test
11+
zap_file: test_full.zap
12+
full: true
13+
zcl_file: zcl_appended.json
14+
clusters:
15+
[../scripts/west/tests/Cluster1.xml, ../scripts/west/tests/Cluster2.xml]

scripts/west/tests/zap_tests.py

Lines changed: 140 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
TEST_OBSOLETE_ZAP_FILE = SCRIPT_DIR / "test_obsolete.zap"
3737
APP_TEMPLATES_FILE = MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH
3838
VERSION_FILE = MATTER_BASE / DEFAULT_ZAP_VERSION_RELATIVE_PATH
39+
TEST_SAMPLES_FILE = SCRIPT_DIR / "zap_samples.yml"
3940

4041

4142
class TestWestZap(unittest.TestCase):
@@ -55,8 +56,10 @@ def setUpClass(cls):
5556
cls.zap_output_dir = cls.test_dir / "zap-generated"
5657
cls.zap_output_dir_full = cls.test_dir / "zap-generated-full"
5758
cls.zap_output_dir_synced = cls.test_dir / "zap-generated-synced"
59+
cls.zap_output_dir_samples_yml = cls.test_dir / "zap-generated-samples-yml"
5860
cls.test_obsolete_zap_file = cls.test_dir / "test_obsolete.zap"
5961
cls.test_obsolete_zcl_file = cls.test_dir / "zcl_test_obsolete.json"
62+
cls.test_samples_file = cls.test_dir / "zap_samples.yml"
6063

6164
cls.cluster_names = [cluster.stem for cluster in cls.test_clusters]
6265

@@ -74,10 +77,14 @@ def setUpClass(cls):
7477
shutil.copy(TEST_ZCL_FILE, cls.zcl_json_file_with_new_items)
7578
shutil.copy(TEST_ZAP_FILE, cls.test_zap_file)
7679
shutil.copy(VERSION_FILE, cls.version_file)
80+
shutil.copy(TEST_SAMPLES_FILE, cls.test_samples_file)
7781

7882
with open(cls.version_file, 'r') as f:
7983
cls.recommended_version = f.read().strip()
8084

85+
# Initialize common zap installer
86+
cls.zap_installer = ZapInstaller(cls.test_dir)
87+
8188
@classmethod
8289
def tearDownClass(cls):
8390
if cls.test_dir.exists():
@@ -155,18 +162,27 @@ def test_post_process_generated_files(self):
155162
with open(test_file, 'w') as f:
156163
f.write("test content")
157164

158-
post_process_generated_files(self.test_dir)
165+
post_process_generated_files(self.test_dir, "manufacturer_specific")
159166

160167
with open(test_file, 'r') as f:
161168
content = f.read()
162169
self.assertTrue(content.endswith('\n'))
163170
self.assertEqual(content, "test content\n")
164171

172+
with open(test_file, 'w') as f:
173+
f.write("# Cluster generated code for constants and metadata based on /home/xxx/ncs/nrf/samples/matter/manufacturer_specific/src/default_zap/manufacturer_specific.matter\n")
174+
f.write("// based on /home/xxx/ncs/nrf/samples/matter/manufacturer_specific/src/default_zap/manufacturer_specific.matter\n")
175+
176+
post_process_generated_files(self.test_dir, "manufacturer_specific")
177+
with open(test_file, 'r') as f:
178+
content = f.readlines()
179+
self.assertEqual(len(content), 0)
180+
165181
# Test file with multiple newlines
166182
with open(test_file, 'w') as f:
167183
f.write("test content\n\n\n")
168184

169-
post_process_generated_files(self.test_dir)
185+
post_process_generated_files(self.test_dir, "manufacturer_specific")
170186

171187
with open(test_file, 'r') as f:
172188
content = f.read()
@@ -215,17 +231,17 @@ def test_get_paths(self):
215231
- The zap CLI path is returned correctly.
216232
"""
217233
# Install path
218-
installer = ZapInstaller(self.test_dir)
234+
installer = self.zap_installer
219235
expected = self.test_dir / '.zap-install'
220236
self.assertEqual(installer.get_install_path(), expected)
221237

222238
# Zap path
223-
installer = ZapInstaller(self.test_dir)
239+
installer = self.zap_installer
224240
expected = self.test_dir / '.zap-install' / installer.zap_exe
225241
self.assertEqual(installer.get_zap_path(), expected)
226242

227243
# Zap CLI path
228-
installer = ZapInstaller(self.test_dir)
244+
installer = self.zap_installer
229245
expected = self.test_dir / '.zap-install' / installer.zap_cli_exe
230246
self.assertEqual(installer.get_zap_cli_path(), expected)
231247

@@ -238,11 +254,11 @@ def test_version(self):
238254
- The current version is returned correctly.
239255
"""
240256

241-
installer = ZapInstaller(self.test_dir)
257+
installer = self.zap_installer
242258
version = installer.get_recommended_version()
243259
self.assertEqual(version, self.recommended_version)
244260

245-
installer = ZapInstaller(self.test_dir)
261+
installer = self.zap_installer
246262

247263
with patch('subprocess.check_output', side_effect=Exception()):
248264
version = installer.get_current_version()
@@ -277,7 +293,7 @@ def test_install_zap(self):
277293
- The ZAP package is not installed if the current ver sion is the same as the recommended version.
278294
- The ZAP package is installed.
279295
"""
280-
zap_installer = ZapInstaller(self.test_dir)
296+
zap_installer = self.zap_installer
281297

282298
# Test when the current version is the same as the recommended version
283299
with patch.object(zap_installer, 'get_current_version', return_value=self.recommended_version):
@@ -314,19 +330,20 @@ def test_zap_generate(self):
314330
- Zap files are generated correctly.
315331
- The data model is re-generated for --full argument and all zap files for new clusters are generated.
316332
"""
317-
zap_installer = ZapInstaller(self.test_dir)
333+
zap_installer = self.zap_installer
318334
self.assertTrue(zap_installer.get_current_version() != "")
319335

320336
# Run zap-generate command for simple generation
321337
with patch('zap_generate.get_zap_generate_path', return_value=MATTER_BASE / DEFAULT_ZAP_GENERATE_RELATIVE_PATH):
322338
with patch('zap_generate.get_app_templates_path', return_value=MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH):
323-
ZapGenerate().do_run(Namespace(zap_file=self.test_zap_file,
324-
output=self.zap_output_dir,
325-
matter_path=self.test_dir,
326-
full=False,
327-
keep_previous=False,
328-
zcl=None,
329-
yaml=None), [])
339+
with patch('zap_generate.ZapInstaller', return_value=zap_installer):
340+
ZapGenerate().do_run(Namespace(zap_file=self.test_zap_file,
341+
output=self.zap_output_dir,
342+
matter_path=self.test_dir,
343+
full=False,
344+
keep_previous=False,
345+
zcl=None,
346+
yaml=None), [])
330347

331348
self.assertTrue(self.zap_output_dir.exists())
332349
self.assertTrue((self.zap_output_dir.parent / "test.matter").exists())
@@ -375,17 +392,112 @@ def test_zap_generate_full(self):
375392
# Use the full zap file to generate the full data model.
376393
with patch('zap_generate.get_zap_generate_path', return_value=MATTER_BASE / DEFAULT_ZAP_GENERATE_RELATIVE_PATH):
377394
with patch('zap_generate.get_app_templates_path', return_value=MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH):
378-
ZapGenerate().do_run(Namespace(zap_file=self.test_zap_file_full,
379-
output=self.zap_output_dir_full,
380-
matter_path=MATTER_BASE,
381-
full=True,
382-
keep_previous=False,
383-
zcl=self.zcl_json_appended,
384-
yaml=None), [])
395+
with patch('zap_generate.ZapInstaller', return_value=self.zap_installer):
396+
ZapGenerate().do_run(Namespace(zap_file=self.test_zap_file_full,
397+
output=self.zap_output_dir_full,
398+
matter_path=MATTER_BASE,
399+
full=True,
400+
keep_previous=False,
401+
zcl=self.zcl_json_appended,
402+
yaml=None), [])
385403

386404
# Check full generation
387405
self._check_full_generation(self.zap_output_dir_full, self.cluster_names)
388406

407+
def test_generate_from_yaml(self):
408+
"""
409+
Checks whether the zap_generate function generates the ZAP package correctly from a yaml file.
410+
"""
411+
412+
# Copy the zap file and zcl to compare later.
413+
zap_to_compare = self.test_dir / "zap_to_comapre.zap"
414+
zcl_to_compare = self.test_dir / "zcl_to_compare.json"
415+
shutil.copy(self.test_zap_file_full, zap_to_compare)
416+
shutil.copy(self.zcl_json_appended, zcl_to_compare)
417+
418+
# Replace the base_dir relative to the ZEPHYR_BASE directory.
419+
with open(self.test_samples_file, 'r') as f:
420+
ZEPHYR_BASE = os.environ.get('ZEPHYR_BASE', "")
421+
samples_yml_content = f.read()
422+
samples_yml_content = samples_yml_content.replace(
423+
"base_dir: ../modules/lib/matter/test_dir", f"base_dir: {self.test_dir.relative_to(Path(ZEPHYR_BASE), walk_up=True)}")
424+
with open(self.test_samples_file, 'w') as f:
425+
f.write(samples_yml_content)
426+
427+
# Run generate using the yaml file
428+
with patch('zap_generate.get_zap_generate_path', return_value=MATTER_BASE / DEFAULT_ZAP_GENERATE_RELATIVE_PATH):
429+
with patch('zap_generate.get_app_templates_path', return_value=MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH):
430+
with patch('zap_generate.ZapInstaller', return_value=self.zap_installer):
431+
ZapGenerate().do_run(Namespace(zap_file=None, output=self.zap_output_dir_samples_yml,
432+
matter_path=MATTER_BASE, full=None, keep_previous=False, zcl=None, yaml=self.test_samples_file), [])
433+
434+
# Check full generation
435+
self._check_full_generation(self.zap_output_dir_samples_yml, self.cluster_names)
436+
437+
# Check whether all generated files are the same as the ones generated from the zap_generate_full test.
438+
failures = []
439+
440+
# Recursively collect all files from both directories
441+
def collect_files(directory):
442+
"""Recursively collect all files in a directory."""
443+
files = {}
444+
for file_path in directory.rglob("*"):
445+
if file_path.is_file():
446+
# Get relative path from the directory root
447+
rel_path = file_path.relative_to(directory)
448+
files[rel_path] = file_path
449+
return files
450+
451+
full_files = collect_files(self.zap_output_dir_full)
452+
samples_yml_files = collect_files(self.zap_output_dir_samples_yml)
453+
454+
# Check all files from zap_output_dir_full
455+
for rel_path, full_file in full_files.items():
456+
samples_yml_file = self.zap_output_dir_samples_yml / rel_path
457+
458+
if rel_path not in samples_yml_files:
459+
failures.append(f"File missing in samples_yml: {rel_path}")
460+
else:
461+
try:
462+
with open(full_file, 'r', encoding='utf-8', errors='ignore') as f:
463+
full_content = f.read()
464+
with open(samples_yml_file, 'r', encoding='utf-8', errors='ignore') as f:
465+
samples_yml_content = f.read()
466+
467+
if full_content != samples_yml_content:
468+
failures.append(f"File content differs: {rel_path}")
469+
except Exception as e:
470+
failures.append(f"Error comparing file {rel_path}: {str(e)}")
471+
472+
# Check for files in samples_yml that are not in full
473+
for rel_path in samples_yml_files:
474+
if rel_path not in full_files:
475+
failures.append(f"Extra file in samples_yml: {rel_path}")
476+
477+
# Print all failures
478+
if failures:
479+
print("\n" + "=" * 80)
480+
print(f"Found {len(failures)} file comparison failure(s):")
481+
print("=" * 80)
482+
for failure in failures:
483+
print(f" - {failure}")
484+
print("=" * 80 + "\n")
485+
486+
# Assert that there are no failures
487+
self.assertEqual(len(failures), 0, f"Found {len(failures)} file comparison failure(s). See output above for details.")
488+
489+
# Compare zap_from_yml.zap with self.test_zap_file_full
490+
with open(zap_to_compare, "rb") as f1, open(self.test_zap_file_full, "rb") as f2:
491+
zap_to_compare_content = f1.read()
492+
zap_full_content = f2.read()
493+
self.assertEqual(zap_to_compare_content, zap_full_content, "zap_to_compare.zap and test_zap_file_full differ")
494+
495+
# Compare zcl_from_yml.json with zcl_json_appended
496+
with open(zcl_to_compare, "rb") as f1, open(self.zcl_json_appended, "rb") as f2:
497+
zcl_to_compare_content = f1.read()
498+
zcl_appended_content = f2.read()
499+
self.assertEqual(zcl_to_compare_content, zcl_appended_content, "zcl_to_compare.json and zcl_json_appended differ")
500+
389501
def test_zap_synchronize(self):
390502
"""
391503
Checks whether the zap_sync function synchronizes the ZAP file correctly.
@@ -399,7 +511,6 @@ def test_zap_synchronize(self):
399511
# Input files should exist.
400512
self.assertTrue(self.zcl_json_appended.exists())
401513
shutil.copy(TEST_ZAP_FILE_FULL, self.test_zap_file_full)
402-
# self.assertTrue(self.test_zap_file_full.exists())
403514

404515
# Copy the obsolete zcl.json file to the test directory.
405516
shutil.copy(TEST_OBSOLETE_ZCL_FILE, self.test_obsolete_zcl_file)
@@ -425,8 +536,9 @@ def test_zap_synchronize(self):
425536
# Run zap-generate command
426537
with patch('zap_generate.get_zap_generate_path', return_value=MATTER_BASE / DEFAULT_ZAP_GENERATE_RELATIVE_PATH):
427538
with patch('zap_generate.get_app_templates_path', return_value=MATTER_BASE / DEFAULT_APP_TEMPLATES_RELATIVE_PATH):
428-
ZapGenerate().do_run(Namespace(zap_file=self.test_obsolete_zap_file, output=self.zap_output_dir_synced,
429-
matter_path=MATTER_BASE, full=True, keep_previous=False, zcl=self.test_obsolete_zcl_file, yaml=None), [])
539+
with patch('zap_generate.ZapInstaller', return_value=self.zap_installer):
540+
ZapGenerate().do_run(Namespace(zap_file=self.test_obsolete_zap_file, output=self.zap_output_dir_synced,
541+
matter_path=MATTER_BASE, full=True, keep_previous=False, zcl=self.test_obsolete_zcl_file, yaml=None), [])
430542

431543
# Check full generation
432544
self._check_full_generation(self.zap_output_dir_synced, self.cluster_names)
@@ -521,7 +633,8 @@ def suite():
521633
'test_zap_generate',
522634
'test_zap_append',
523635
'test_zap_generate_full',
524-
'test_zap_synchronize'
636+
'test_generate_from_yaml',
637+
# 'test_zap_synchronize',
525638
]
526639

527640
loader = unittest.TestLoader()

scripts/west/zap_common.py

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ def update_zcl_in_zap(zap_file: Path, zcl_json: Path, app_templates: Path) -> bo
120120
Functions returns True if the path was updated, False otherwise.
121121
"""
122122
updated = False
123+
zap_file = zap_file.resolve()
124+
zcl_json = zcl_json.resolve()
125+
app_templates = app_templates.resolve()
123126

124127
with open(zap_file, 'r+') as file:
125128
data = json.load(file)
@@ -153,13 +156,15 @@ def update_zcl_in_zap(zap_file: Path, zcl_json: Path, app_templates: Path) -> bo
153156
return updated
154157

155158

156-
def post_process_generated_files(output_path: Path):
159+
def post_process_generated_files(output_path: Path, base_dir: str):
157160
"""
158161
Post-process the generated files:
159162
160163
- Decode as utf-8, fallback to system default if needed
161164
- Ensure all files in output_path (recursively) have exactly one empty line at the end
162165
- If some files contains path to the local files, remove the absolute paths
166+
167+
- The base_dir is used to find and clear all absolute paths to the local files.
163168
"""
164169
for root, _, files in os.walk(output_path):
165170
for fname in files:
@@ -187,17 +192,11 @@ def post_process_generated_files(output_path: Path):
187192
# Check if the file contains absolute paths to .matter files
188193
lines = new_text.splitlines()
189194
for i, line in zip(range(20), lines):
190-
# Check if line contains "// based on" pattern with absolute path
191-
if '// based on' in line:
192-
# Find absolute paths to .matter files using regex
193-
# Pattern matches the entire absolute path ending with .matter
194-
pattern = r'(// based on .*?)(nrf/.*?\.matter)'
195-
match = re.search(pattern, line)
196-
if match:
197-
# Replace the entire line part with just "// based on " + relative path
198-
absolute_part = match.group(1)
199-
relative_path = match.group(2)
200-
new_text = new_text.replace(line, "// based on " + relative_path)
195+
# Check if line contains "based on" pattern with absolute path
196+
if 'based on' in line:
197+
# Remove the line if it contains '// based on' or '# based on'
198+
if line.strip().startswith("// based on") or line.strip().startswith("# Cluster generated code for constants and metadata based on" or line.strip().startswith("# List of cluster")):
199+
new_text = new_text.replace(line + '\n', '')
201200

202201
if new_text != text:
203202
with open(file_path, 'w', encoding='utf-8') as f:

0 commit comments

Comments
 (0)