From: Simon Glass <simon.glass@canonical.com> Several series subcommands complete silently or only log at the info level, which is not visible with the default NOTICE log level. Add a tout.notice() summary to each action subcommand so the user sees confirmation of what was done. For subcommands that already have a summary at info level, promote it to notice. For those with no summary at all, add one. Also update tests to suppress the new output where needed. Co-developed-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/patman/cseries.py | 40 ++++++++++++----- tools/patman/test_cseries.py | 85 +++++++++++++++++++++++------------- 2 files changed, 83 insertions(+), 42 deletions(-) diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py index 937af58ba3e..c90020b7eb6 100644 --- a/tools/patman/cseries.py +++ b/tools/patman/cseries.py @@ -142,7 +142,7 @@ class Cseries(cser_helper.CseriesHelper): del_branch = repo.lookup_branch(del_name) branch_oid = del_branch.peel(pygit2.enums.ObjectType.COMMIT).oid del_branch.delete() - print(f"Deleted branch '{del_name}' {oid(branch_oid)}") + tout.info(f"Deleted branch '{del_name}' {oid(branch_oid)}") self.db.ser_ver_remove(ser.idnum, max_vers) if not dry_run: @@ -150,6 +150,8 @@ class Cseries(cser_helper.CseriesHelper): else: self.rollback() + tout.notice(f"Decremented series '{ser.name}' to v{new_max}") + def increment(self, series_name, dry_run=False): """Increment a series to the next version and create a new branch @@ -191,7 +193,7 @@ class Cseries(cser_helper.CseriesHelper): self.rollback() # repo.head.set_target(amended) - tout.info(f'Added new branch {new_name}') + tout.notice(f"Incremented series '{ser.name}' to v{vers}") if dry_run: tout.info('Dry run completed') @@ -372,7 +374,7 @@ class Cseries(cser_helper.CseriesHelper): msg += f', {no_desc} missing description' if failed: msg += f', {failed} updated failed' - tout.info(msg + f' ({requests} requests)') + tout.notice(msg + f' ({requests} requests)') tout.info('') tout.info(f"{'Name':15} Version {'Description':40} Result") @@ -466,6 +468,9 @@ class Cseries(cser_helper.CseriesHelper): f'Marked commits {len(bad)}/{len(ser.commits)}') new_oid = self._mark_series(in_name, ser, dry_run=dry_run) + count = len(ser.commits) + tout.notice(f"Marked {count} commit{self.plural(count)}" + f" in series '{name}'") if dry_run: tout.info('Dry run completed') return new_oid @@ -510,6 +515,9 @@ class Cseries(cser_helper.CseriesHelper): else: vals.info = 'no mark' + count = len(ser.commits) + tout.notice(f"Unmarked {count} commit{self.plural(count)}" + f" in series '{name}'") if dry_run: tout.info('Dry run completed') return vals.oid @@ -756,7 +764,7 @@ class Cseries(cser_helper.CseriesHelper): else: self.rollback() - tout.info(f"Renamed series '{series}' to '{name}'") + tout.notice(f"Renamed series '{series}' to '{name}'") if dry_run: tout.info('Dry run completed') @@ -916,6 +924,9 @@ class Cseries(cser_helper.CseriesHelper): self.db.series_set_archived(ser.idnum, True) self.commit() + count = len(tag_info) + tout.notice(f"Archived series '{ser.name}'" + f" ({count} version{self.plural(count)})") def unarchive(self, series): """Unarchive a series @@ -958,6 +969,9 @@ class Cseries(cser_helper.CseriesHelper): self.db.ser_ver_set_archive_tag(idnum, None) self.commit() + count = len(tag_info) + tout.notice(f"Unarchived series '{ser.name}'" + f" ({count} version{self.plural(count)})") def status(self, pwork, series, version, show_comments, show_cover_comments=False): @@ -1029,9 +1043,9 @@ class Cseries(cser_helper.CseriesHelper): updated, updated_cover = self._sync_one( svid, ser.name, version, show_comments, show_cover_comments, gather_tags, cover, patches, dry_run) - tout.info(f"{updated} patch{'es' if updated != 1 else ''}" - f"{' and cover letter' if updated_cover else ''} " - f'updated ({stats.request_count} requests)') + tout.notice(f"{updated} patch{'es' if updated != 1 else ''}" + f"{' and cover letter' if updated_cover else ''} " + f'updated ({stats.request_count} requests)') if not dry_run: self.commit() @@ -1064,7 +1078,7 @@ class Cseries(cser_helper.CseriesHelper): add_newline = gather_tags tout.info('') - tout.info( + tout.notice( f"{tot_updated} patch{'es' if tot_updated != 1 else ''} and " f"{tot_cover} cover letter{'s' if tot_cover != 1 else ''} " f'updated, {missing} missing ' @@ -1084,6 +1098,7 @@ class Cseries(cser_helper.CseriesHelper): """ self.db.upstream_add(name, url) self.commit() + tout.notice(f"Added upstream '{name}' ({url})") def upstream_list(self): """List the upstream repos @@ -1106,6 +1121,8 @@ class Cseries(cser_helper.CseriesHelper): """ self.db.upstream_set_default(name) self.commit() + if name: + tout.notice(f"Set default upstream to '{name}'") def upstream_get_default(self): """Get the default upstream target @@ -1123,6 +1140,7 @@ class Cseries(cser_helper.CseriesHelper): """ self.db.upstream_delete(name) self.commit() + tout.notice(f"Deleted upstream '{name}'") def version_remove(self, name, version, dry_run=False): """Remove a version of a series from the database @@ -1147,7 +1165,7 @@ class Cseries(cser_helper.CseriesHelper): else: self.rollback() - tout.info(f"Removed version {version} from series '{name}'") + tout.notice(f"Removed version {version} from series '{name}'") if dry_run: tout.info('Dry run completed') @@ -1194,7 +1212,7 @@ class Cseries(cser_helper.CseriesHelper): else: self.rollback() - tout.info(f"Changed version {version} in series '{ser.name}' " - f"to {new_version} named '{new_name}'") + tout.notice(f"Changed version {version} in series '{ser.name}' " + f"to {new_version} named '{new_name}'") if dry_run: tout.info('Dry run completed') diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py index 3aa8e0e739e..c8af0bb1db7 100644 --- a/tools/patman/test_cseries.py +++ b/tools/patman/test_cseries.py @@ -772,7 +772,8 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak def test_series_list_archived(self): """Archive a series and test listing it""" self.setup_second() - self.cser.archive('first') + with terminal.capture(): + self.cser.archive('first') with terminal.capture() as (out, _): self.run_args('series', 'ls', pwork=True) lines = out.getvalue().splitlines() @@ -877,7 +878,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak f'- add v2: {HASH_RE} as {HASH_RE} spi: SPI fixes') self.assertRegex( next(itr), f'Updating branch first2 from {HASH_RE} to {HASH_RE}') - self.assertEqual('Added new branch first2', next(itr)) + self.assertEqual("Incremented series 'first' to v2", next(itr)) return itr def test_series_link(self): @@ -1532,9 +1533,11 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser = next(cor) # Archive it and make sure it is invisible - cser.archive('first') + with terminal.capture(): + cser.archive('first') cser = next(cor) - cser.unarchive('first') + with terminal.capture(): + cser.unarchive('first') self.assertFalse(next(cor)) cor.close() @@ -1544,11 +1547,13 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser = next(cor) # Archive it and make sure it is invisible - self.run_args('series', '-s', 'first', 'archive', pwork=True, - cser=cser) + with terminal.capture(): + self.run_args('series', '-s', 'first', 'archive', pwork=True, + cser=cser) next(cor) - self.run_args('series', '-s', 'first', 'unarchive', pwork=True, - cser=cser) + with terminal.capture(): + self.run_args('series', '-s', 'first', 'unarchive', pwork=True, + cser=cser) self.assertFalse(next(cor)) cor.close() @@ -1696,10 +1701,11 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak with terminal.capture() as (out, _): cser.decrement('first') lines = out.getvalue().splitlines() - self.assertEqual(2, len(lines)) + self.assertEqual(3, len(lines)) self.assertEqual("Removing series 'first' v2", lines[0]) self.assertEqual( f"Deleted branch 'first2' {str(branch_oid)[:10]}", lines[1]) + self.assertEqual("Decremented series 'first' to v1", lines[2]) svdict = cser.get_ser_ver_dict() self.assertEqual(1, len(svdict)) @@ -1720,12 +1726,14 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak """Test adding an upsream""" cser = self.get_cser() - cser.upstream_add('us', 'https://one') + with terminal.capture(): + cser.upstream_add('us', 'https://one') ulist = cser.get_upstream_dict() self.assertEqual(1, len(ulist)) self.assertEqual(('https://one', None), ulist['us']) - cser.upstream_add('ci', 'git@two') + with terminal.capture(): + cser.upstream_add('ci', 'git@two') ulist = cser.get_upstream_dict() self.assertEqual(2, len(ulist)) self.assertEqual(('https://one', None), ulist['us']) @@ -1762,17 +1770,21 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser.upstream_set_default('us') self.assertEqual("No such upstream 'us'", str(exc.exception)) - cser.upstream_add('us', 'https://one') - cser.upstream_add('ci', 'git@two') + with terminal.capture(): + cser.upstream_add('us', 'https://one') + cser.upstream_add('ci', 'git@two') self.assertIsNone(cser.upstream_get_default()) - cser.upstream_set_default('us') + with terminal.capture(): + cser.upstream_set_default('us') self.assertEqual('us', cser.upstream_get_default()) - cser.upstream_set_default('us') + with terminal.capture(): + cser.upstream_set_default('us') - cser.upstream_set_default('ci') + with terminal.capture(): + cser.upstream_set_default('ci') self.assertEqual('ci', cser.upstream_get_default()) with terminal.capture() as (out, _): @@ -1792,19 +1804,22 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertEqual("patman: ValueError: No such upstream 'us'", out.getvalue().strip().splitlines()[-1]) - self.run_args('upstream', 'add', 'us', 'https://one') - self.run_args('upstream', 'add', 'ci', 'git@two') + with terminal.capture(): + self.run_args('upstream', 'add', 'us', 'https://one') + self.run_args('upstream', 'add', 'ci', 'git@two') with terminal.capture() as (out, _): self.run_args('upstream', 'default') self.assertEqual('unset', out.getvalue().strip()) - self.run_args('upstream', 'default', 'us') + with terminal.capture(): + self.run_args('upstream', 'default', 'us') with terminal.capture() as (out, _): self.run_args('upstream', 'default') self.assertEqual('us', out.getvalue().strip()) - self.run_args('upstream', 'default', 'ci') + with terminal.capture(): + self.run_args('upstream', 'default', 'ci') with terminal.capture() as (out, _): self.run_args('upstream', 'default') self.assertEqual('ci', out.getvalue().strip()) @@ -1825,14 +1840,17 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak cser.upstream_delete('us') self.assertEqual("No such upstream 'us'", str(exc.exception)) - cser.upstream_add('us', 'https://one') - cser.upstream_add('ci', 'git@two') + with terminal.capture(): + cser.upstream_add('us', 'https://one') + cser.upstream_add('ci', 'git@two') - cser.upstream_set_default('us') - cser.upstream_delete('us') + with terminal.capture(): + cser.upstream_set_default('us') + cser.upstream_delete('us') self.assertIsNone(cser.upstream_get_default()) - cser.upstream_delete('ci') + with terminal.capture(): + cser.upstream_delete('ci') ulist = cser.get_upstream_dict() self.assertFalse(ulist) @@ -1843,17 +1861,20 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak self.assertEqual("patman: ValueError: No such upstream 'us'", out.getvalue().strip().splitlines()[-1]) - self.run_args('us', 'add', 'us', 'https://one') - self.run_args('us', 'add', 'ci', 'git@two') + with terminal.capture(): + self.run_args('us', 'add', 'us', 'https://one') + self.run_args('us', 'add', 'ci', 'git@two') - self.run_args('upstream', 'default', 'us') - self.run_args('upstream', 'delete', 'us') + with terminal.capture(): + self.run_args('upstream', 'default', 'us') + self.run_args('upstream', 'delete', 'us') with terminal.capture() as (out, _): self.run_args('upstream', 'default', 'us', expect_ret=1) self.assertEqual("patman: ValueError: No such upstream 'us'", out.getvalue().strip()) - self.run_args('upstream', 'delete', 'ci') + with terminal.capture(): + self.run_args('upstream', 'delete', 'ci') with terminal.capture() as (out, _): self.run_args('upstream', 'list') self.assertFalse(out.getvalue().strip()) @@ -2003,6 +2024,7 @@ Tested-by: Mary Smith <msmith@wibble.com> # yak f'- unmarked: {HASH_RE} as {HASH_RE} spi: SPI fixes') self.assertRegex( next(itr), f'Updating branch first from {HASH_RE} to {HASH_RE}') + self.assertEqual("Unmarked 2 commits in series 'first'", next(itr)) self.assertEqual('Dry run completed', next(itr)) with self.stage('unmark'): @@ -3086,7 +3108,8 @@ Date: .* def test_series_progress_all_archived(self): """Test showing progress for all cseries including archived ones""" self.setup_second() - self.cser.archive('first') + with terminal.capture(): + self.cser.archive('first') with self.stage('progress without archived'): with terminal.capture() as (out, _): -- 2.43.0