From a82a5b7a5f8a89f5cb55f1228ebc7c08e3657497 Mon Sep 17 00:00:00 2001 From: Brigs Date: Sun, 20 Sep 2026 09:52:29 -0400 Subject: [PATCH] Let an empty rows= iterable leave no LAVA table add_row() registers the artifact's table at the first row, so a streaming module that finds nothing leaves nothing. The rows= form had no such guarantee: __bool__ returned True for any non-list iterable, so the core registered the table and a manifest entry before a row proved there was data, and an artifact that found nothing came out as a table with no rows and a zero record count. In a report that reads as "parsed, found nothing" rather than "did not run". __bool__ now takes a one-item lookahead and __iter__ puts the item back, so both forms behave alike and consuming the result still yields every row. Nothing in the tree uses rows= today; this closes the trap before anything does. Tests cover the empty iterable, the empty list, a non-empty iterable keeping every row after the lookahead, asking twice, and both shapes through artifact_processor. Two of them fail without the change. Co-Authored-By: Claude Opus 5 --- admin/test/scripts/test_artifact_result.py | 72 ++++++++++++++++++++++ leapp_functions/app/artifact_result.py | 28 ++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) diff --git a/admin/test/scripts/test_artifact_result.py b/admin/test/scripts/test_artifact_result.py index 51a19845..b100fc5c 100644 --- a/admin/test/scripts/test_artifact_result.py +++ b/admin/test/scripts/test_artifact_result.py @@ -218,6 +218,16 @@ def test_the_context_hands_back_the_result_it_created(self): 'author': '@unit', 'creation_date': '2026-09-20', 'last_update_date': '2026-09-20', 'notes': '', 'paths': ('*/nothing',), 'output_types': ['lava'], }, + 'rows_iterable_module_empty': { + 'name': 'Rows Iterable Empty', 'category': 'Probe Category', 'description': 'probe', + 'author': '@unit', 'creation_date': '2026-09-20', 'last_update_date': '2026-09-20', + 'notes': '', 'paths': ('*/nothing',), 'output_types': ['lava'], + }, + 'rows_iterable_module_rows': { + 'name': 'Rows Iterable Rows', 'category': 'Probe Category', 'description': 'probe', + 'author': '@unit', 'creation_date': '2026-09-20', 'last_update_date': '2026-09-20', + 'notes': '', 'paths': ('*/nothing',), 'output_types': ['lava'], + }, } @@ -321,3 +331,65 @@ def test_python_text_restore_is_exact_and_leaves_other_text_alone(self): self.assertEqual(restore('nan'), 'nan') # non-finite never converts self.assertEqual(restore('["a", "b"]', 'media'), ['a', 'b']) self.assertEqual(restore('single-ref', 'media'), 'single-ref') + +@artifact_processor +def rows_iterable_module_empty(context): + return context.create_artifact_result(headers=HEADERS, source_path='probe.db', rows=iter(())) + + +@artifact_processor +def rows_iterable_module_rows(context): + return context.create_artifact_result(headers=HEADERS, source_path='probe.db', rows=iter(ROWS)) + + +class TestRowsIterableEmptyCase(ArtifactResultTestCase): + """An iterable passed with rows= that yields nothing must leave no table and no manifest + entry, the same as add_row() with no rows, and a non-empty one must keep every row.""" + + def setUp(self): + super().setUp() + lavafuncs.initialize_lava(self.tmpdir, self.tmpdir, 'fs') + self._saved_log_paths = {name: getattr(OutputParameters, name) for name in vars(OutputParameters) + if name.startswith('screen_output_file_path')} + Context.set_output_params(OutputParameters(self.tmpdir)) + + def tearDown(self): + for name, value in self._saved_log_paths.items(): + setattr(OutputParameters, name, value) + super().tearDown() + + def _artifact_tables(self): + cursor = lavafuncs.lava_db.cursor() + return [r[0] for r in cursor.execute("SELECT name FROM sqlite_master WHERE type='table'") + if not r[0].startswith('_')] + + def test_an_empty_iterable_is_falsey(self): + self.assertFalse(ArtifactResult(headers=HEADERS, rows=iter(()))) + + def test_an_empty_list_is_still_falsey(self): + self.assertFalse(ArtifactResult(headers=HEADERS, rows=[])) + + def test_a_non_empty_iterable_is_truthy_and_keeps_every_row(self): + result = ArtifactResult(headers=HEADERS, rows=iter(ROWS)) + self.assertTrue(result) + # The lookahead pulled the first row; iterating has to put it back. + self.assertEqual(list(result), list(ROWS)) + + def test_asking_twice_does_not_lose_the_first_row(self): + result = ArtifactResult(headers=HEADERS, rows=iter(ROWS)) + self.assertTrue(result) + self.assertTrue(result) + self.assertEqual(list(result), list(ROWS)) + + def test_an_empty_iterable_module_leaves_no_table_and_no_manifest_entry(self): + rows_iterable_module_empty([], self.tmpdir, None, True) # pylint: disable=too-many-function-args + self.assertEqual(self._artifact_tables(), []) + self.assertNotIn('Probe Category', lavafuncs.lava_data['artifacts']) + + def test_a_non_empty_iterable_module_writes_every_row(self): + rows_iterable_module_rows([], self.tmpdir, None, True) # pylint: disable=too-many-function-args + tables = self._artifact_tables() + self.assertEqual(len(tables), 1) + count = lavafuncs.lava_db.execute( + f'SELECT COUNT(*) FROM {lavafuncs.quote_sql_name(tables[0])}').fetchone()[0] + self.assertEqual(count, len(ROWS)) diff --git a/leapp_functions/app/artifact_result.py b/leapp_functions/app/artifact_result.py index d693ff64..7719617f 100644 --- a/leapp_functions/app/artifact_result.py +++ b/leapp_functions/app/artifact_result.py @@ -25,6 +25,11 @@ result, so a writer-style module behaves like one that returned a list. """ +import itertools + +_UNSET = object() +_NO_ROWS = object() + class ArtifactResult: """Container for artifact metadata and rows to be streamed by the core.""" @@ -59,6 +64,7 @@ def __init__( self._column_map = None self._is_lava_backed = False self._buffer = None + self._peeked = _UNSET def _format_source_path(self, source_path): if source_path and self._source_path_formatter: @@ -255,9 +261,27 @@ def __bool__(self): return False if isinstance(self._rows, list): return bool(self._rows) - return True + # An iterable passed with rows= may yield nothing, and the core decides whether to + # register a LAVA table from this result's truth value. Without a lookahead every + # iterable is truthy, so an artifact that found nothing would leave an empty table + # and a zero-count manifest entry, which reads as "parsed, found nothing" rather + # than "did not run". add_row() already defers registration to the first row; this + # gives the rows= form the same guarantee. The lookahead runs once and __iter__ + # puts the item back, so consuming the result still yields every row. + return self._peek() is not _NO_ROWS + + def _peek(self): + """Pull and hold the first row of a rows= iterable, once.""" + if self._peeked is _UNSET: + rows = iter(self._rows) + self._peeked = next(rows, _NO_ROWS) + self._rows = rows + return self._peeked def __iter__(self): if self._buffer is not None: return iter(self._buffer) - return iter(self._rows) + if self._peeked is _UNSET or self._peeked is _NO_ROWS: + return iter(self._rows) + first, self._peeked = self._peeked, _UNSET + return itertools.chain([first], self._rows)