Several PRs in the current Classic-parity sweep add argparse-layer tests
but no coverage of the load-bearing logic in run_command where the
parsed args actually get applied to TSC items or normalized. This is a
common pattern across the sweep, not a single-PR issue, so recording
here as a follow-up rather than blocking those PRs.
Concrete gaps
#451 delete syntax normalization. run_command transforms
isinstance(args.workbook, str) -> args.name = args.workbook; args.workbook = True (and same for datasource). This is what makes both
delete Name --workbook and Classic delete --workbook Name produce
equivalent behavior. No test covers the transform; both syntaxes are
verified at the parser layer only. If someone refactors run_command
and drops the normalization, every parser test still passes but Classic
scripts break silently.
#452 editsite attribute application. run_command reads ten new
argparse fields onto SiteItem attributes with three separate patterns:
_str_to_bool coercion for the string-value flags, direct assignment
for the paired boolean flags, and inversion for disable_subscriptions
(from allow_subscriptions). The paired custom_subscription_email +
custom_subscription_email_enabled writes on --subscription-email
(where empty string flips _enabled off) also live in run_command.
None of this is exercised by a mocked server.sites.update call. A
future refactor could drop any of these assignments and no test would
catch it.
#447 creategroup --role. run_command sets
GroupItem.minimum_site_role = args.role before
server.groups.create. Not tested; someone could delete the assignment
and every parser test still passes.
#448 publish --description. run_command sets
new_item.description = args.description on the WorkbookItem or
DatasourceItem before publish. Not tested; same failure mode.
#444 --idp-configuration-id. run_command sets
user_obj.idp_configuration_id = args.idp_configuration_id on every
UserItem in the batch, and apply_cli_overrides clears any pre-existing
auth_setting when the IDP flag is passed. Only the
apply_cli_overrides helper is tested; nothing verifies the attribute
actually reaches the wire.
Suggested shape
Adopt a pattern like tests/commands/test_create_site_users.py uses:
mock the Session/server, mock server.<endpoint>.<method>, invoke
Command.run_command(args), assert the item passed to
server.<endpoint>.<method> has the expected attributes set. Not
required per-flag -- one test per PR covering the main happy path is
enough to lock in the wire-level behavior.
Not tracking here
Several PRs in the current Classic-parity sweep add argparse-layer tests
but no coverage of the load-bearing logic in
run_commandwhere theparsed args actually get applied to TSC items or normalized. This is a
common pattern across the sweep, not a single-PR issue, so recording
here as a follow-up rather than blocking those PRs.
Concrete gaps
#451 delete syntax normalization.
run_commandtransformsisinstance(args.workbook, str) -> args.name = args.workbook; args.workbook = True(and same for datasource). This is what makes bothdelete Name --workbookand Classicdelete --workbook Nameproduceequivalent behavior. No test covers the transform; both syntaxes are
verified at the parser layer only. If someone refactors
run_commandand drops the normalization, every parser test still passes but Classic
scripts break silently.
#452 editsite attribute application.
run_commandreads ten newargparse fields onto
SiteItemattributes with three separate patterns:_str_to_boolcoercion for the string-value flags, direct assignmentfor the paired boolean flags, and inversion for
disable_subscriptions(from
allow_subscriptions). The pairedcustom_subscription_email+custom_subscription_email_enabledwrites on--subscription-email(where empty string flips
_enabledoff) also live inrun_command.None of this is exercised by a mocked
server.sites.updatecall. Afuture refactor could drop any of these assignments and no test would
catch it.
#447 creategroup --role.
run_commandsetsGroupItem.minimum_site_role = args.rolebeforeserver.groups.create. Not tested; someone could delete the assignmentand every parser test still passes.
#448 publish --description.
run_commandsetsnew_item.description = args.descriptionon the WorkbookItem orDatasourceItem before publish. Not tested; same failure mode.
#444 --idp-configuration-id.
run_commandsetsuser_obj.idp_configuration_id = args.idp_configuration_idon everyUserItem in the batch, and
apply_cli_overridesclears any pre-existingauth_settingwhen the IDP flag is passed. Only theapply_cli_overrideshelper is tested; nothing verifies the attributeactually reaches the wire.
Suggested shape
Adopt a pattern like
tests/commands/test_create_site_users.pyuses:mock the Session/server, mock
server.<endpoint>.<method>, invokeCommand.run_command(args), assert the item passed toserver.<endpoint>.<method>has the expected attributes set. Notrequired per-flag -- one test per PR covering the main happy path is
enough to lock in the wire-level behavior.
Not tracking here
delete --workbook Namesyntax #451 bare--workbookwith no value: parser accepts, run_commanderrors out. Already covered at the parser layer; run_command's error
message is standard.
tested elsewhere.
(
test_create_site_users.py); the gap is elsewhere.