forked from vhaudiquet/weekly-activity
feat(config): menu-driven multi-account wizard, v2 schema, config-time Launchpad OAuth
Address review of PR #1 (Valentin): replace the linear config walk-through and single-section schema. - weekly-activity config becomes a menu loop: Current sources (masked) -> Add a new source (pick kind, repeatable) / Remove a source / Exit; the file is written only when something changed - new versioned config layout (version = 2): [[kind.accounts]] arrays so several accounts or instances per provider coexist; optional display labels; strict unknown-key/type validation kept (errors carry account position, e.g. [github.accounts[1]]) - legacy v1 files (no version key, one section per source) migrate transparently in memory on load; saving from the wizard persists v2; the file itself is never rewritten by load_config - non-anonymous Launchpad accounts now authenticate at config time: constructing LaunchpadSource runs launchpadlib's browser OAuth and the token lands in the system keyring before any report; an explicit credentials_file stays supported as an additive opt-out - report aggregates every configured account in stable order with failure isolation intact; headings add the account label when it disambiguates (source_label shared between aggregate and wizard) - GitHub accounts gain optional token_env resolved lazily against the environment before falling back to GITHUB_TOKEN / `gh auth token`
This commit is contained in:
+81
-13
@@ -48,29 +48,90 @@ def window() -> tuple[datetime, datetime]:
|
||||
class BuildSpecsTest(unittest.TestCase):
|
||||
def test_maps_every_section_in_order(self) -> None:
|
||||
config = ActivityConfig(
|
||||
launchpad=LaunchpadSettings(username="lp"),
|
||||
github=GithubSettings(username="gh"),
|
||||
gitlab=GitlabSettings(username="gl"),
|
||||
bts=BtsSettings(email="bts@mail"),
|
||||
launchpad=[LaunchpadSettings(username="lp")],
|
||||
github=[GithubSettings(username="gh")],
|
||||
gitlab=[GitlabSettings(username="gl")],
|
||||
bts=[BtsSettings(email="bts@mail")],
|
||||
)
|
||||
specs = build_specs(config)
|
||||
self.assertEqual([s.name for s in specs], ["launchpad", "github", "gitlab", "bts"])
|
||||
self.assertEqual([s.username for s in specs], ["lp", "gh", "gl", "bts@mail"])
|
||||
|
||||
def test_multiple_accounts_per_kind_in_listing_order(self) -> None:
|
||||
config = ActivityConfig(
|
||||
launchpad=[
|
||||
LaunchpadSettings(username="lp-a"),
|
||||
LaunchpadSettings(username="lp-b", mode="credentials"),
|
||||
],
|
||||
github=[
|
||||
GithubSettings(username="gh-a", name="work"),
|
||||
GithubSettings(username="gh-b"),
|
||||
],
|
||||
bts=[
|
||||
BtsSettings(email="one@x.org"),
|
||||
BtsSettings(email="two@x.org", name="two"),
|
||||
],
|
||||
)
|
||||
specs = build_specs(config)
|
||||
# All launchpad accounts first, then github, then bts; listing order kept.
|
||||
self.assertEqual([s.username for s in specs], ["lp-a", "lp-b", "gh-a", "gh-b",
|
||||
"one@x.org", "two@x.org"])
|
||||
self.assertEqual(
|
||||
[s.name for s in specs],
|
||||
[
|
||||
"launchpad/lp-a", # siblings force identity into the label
|
||||
"launchpad/lp-b",
|
||||
"github/work", # explicit label wins even among siblings
|
||||
"github/gh-b",
|
||||
"bts/one@x.org",
|
||||
"bts/two",
|
||||
],
|
||||
)
|
||||
|
||||
def test_single_unnamed_account_keeps_plain_kind_label(self) -> None:
|
||||
config = ActivityConfig(github=[GithubSettings(username="only-one")])
|
||||
specs = build_specs(config)
|
||||
self.assertEqual(specs[0].name, "github")
|
||||
|
||||
def test_disabled_sections_are_skipped(self) -> None:
|
||||
config = ActivityConfig(github=GithubSettings(username="only-one"))
|
||||
self.assertEqual([s.name for s in build_specs(config)], ["github"])
|
||||
config = ActivityConfig(bts=[BtsSettings(email="only@one")])
|
||||
self.assertEqual([s.name for s in build_specs(config)], ["bts"])
|
||||
|
||||
def test_launchpad_spec_expands_tilde_credentials_file(self) -> None:
|
||||
settings = LaunchpadSettings(
|
||||
username="u", mode="credentials", credentials_file="~/creds.json"
|
||||
)
|
||||
spec = build_specs(ActivityConfig(launchpad=settings))[0]
|
||||
config = ActivityConfig(launchpad=[LaunchpadSettings(username="anon")])
|
||||
config.launchpad.append(LaunchpadSettings(mode="credentials", username="u",
|
||||
credentials_file="~/creds.json"))
|
||||
specs = build_specs(config)
|
||||
with mock.patch("weekly_activity.aggregate.LaunchpadSource") as fake_source:
|
||||
for spec in specs:
|
||||
spec.make()
|
||||
anon_kwargs, cred_kwargs = (call.kwargs for call in fake_source.call_args_list)
|
||||
self.assertTrue(anon_kwargs["anonymous"])
|
||||
self.assertIsNone(anon_kwargs["credentials_file"])
|
||||
self.assertFalse(cred_kwargs["anonymous"])
|
||||
self.assertEqual(cred_kwargs["credentials_file"], os.path.expanduser("~") + "/creds.json")
|
||||
# The two accounts must not share one late-bound factory.
|
||||
self.assertNotEqual(anon_kwargs["credentials_file"], cred_kwargs["credentials_file"])
|
||||
|
||||
def test_github_token_env_resolved_lazily(self) -> None:
|
||||
config = ActivityConfig(github=[GithubSettings(username="u", token_env="MY_GH_TOKEN")])
|
||||
spec = build_specs(config)[0]
|
||||
with (
|
||||
mock.patch("weekly_activity.aggregate.GitHubSource") as fake_source,
|
||||
mock.patch.dict(os.environ, {"MY_GH_TOKEN": "env-token"}),
|
||||
):
|
||||
spec.make()
|
||||
kwargs = fake_source.call_args.kwargs
|
||||
self.assertEqual(kwargs["credentials_file"], os.path.expanduser("~") + "/creds.json")
|
||||
self.assertFalse(kwargs["anonymous"])
|
||||
self.assertEqual(fake_source.call_args.kwargs["token"], "env-token")
|
||||
|
||||
def test_github_stored_token_takes_precedence_over_env(self) -> None:
|
||||
config = ActivityConfig(github=[GithubSettings(username="u", token="tok")])
|
||||
spec = build_specs(config)[0]
|
||||
with (
|
||||
mock.patch("weekly_activity.aggregate.GitHubSource") as fake_source,
|
||||
mock.patch.dict(os.environ, {"MY_GH_TOKEN": "env-token"}),
|
||||
):
|
||||
spec.make()
|
||||
self.assertEqual(fake_source.call_args.kwargs["token"], "tok")
|
||||
|
||||
|
||||
class CollectAndFormatTest(unittest.TestCase):
|
||||
@@ -89,6 +150,13 @@ class CollectAndFormatTest(unittest.TestCase):
|
||||
self.assertIn("Activity report — alpha / ~a", text)
|
||||
self.assertIn("[skipped: beta — RuntimeError: kaput]", text)
|
||||
|
||||
def test_heading_uses_account_label_not_bare_source_name(self) -> None:
|
||||
since, until = window()
|
||||
specs = [SourceSpec(name="github/work", username="acme-jane",
|
||||
make=lambda: StubSource("github"))]
|
||||
text = format_combined(collect_specs(specs, since, until), since, until)
|
||||
self.assertIn("Activity report — github/work / ~acme-jane", text)
|
||||
|
||||
def test_warning_surfaced_inside_block(self) -> None:
|
||||
since, until = window()
|
||||
specs = [SourceSpec(name="gamma", username="g", make=lambda: StubSource("gamma"))]
|
||||
|
||||
+166
-38
@@ -21,54 +21,93 @@ from weekly_activity.config import (
|
||||
)
|
||||
|
||||
|
||||
class ConfigRoundTripTest(unittest.TestCase):
|
||||
def test_round_trip_preserves_all_sections(self) -> None:
|
||||
config = ActivityConfig(
|
||||
launchpad=LaunchpadSettings(
|
||||
def load_config_from_text(text: str) -> ActivityConfig:
|
||||
"""Helper: parse TOML through the public loader without touching disk."""
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
path = Path(tmp) / "weekly-activity.toml"
|
||||
path.write_text(text, encoding="utf-8")
|
||||
return load_config(path)
|
||||
|
||||
|
||||
def sample_v2_config() -> ActivityConfig:
|
||||
return ActivityConfig(
|
||||
launchpad=[
|
||||
LaunchpadSettings(
|
||||
username="lp-user",
|
||||
mode="credentials",
|
||||
credentials_file="/tmp/lp-creds.json",
|
||||
service="staging",
|
||||
name="work",
|
||||
),
|
||||
github=GithubSettings(username='gh"user', token="tok\\en"),
|
||||
gitlab=GitlabSettings(
|
||||
LaunchpadSettings(username="anon-lp"),
|
||||
],
|
||||
github=[
|
||||
GithubSettings(username='gh"user', token="tok\\en"),
|
||||
GithubSettings(
|
||||
username="acme-jane", token_env="WORK_GH_TOKEN", name="work"
|
||||
),
|
||||
],
|
||||
gitlab=[
|
||||
GitlabSettings(
|
||||
username="gl-user",
|
||||
url="https://salsa.debian.org",
|
||||
token_env="SALSA_TOKEN",
|
||||
),
|
||||
bts=BtsSettings(email="dev@example.org"),
|
||||
)
|
||||
name="salsa",
|
||||
)
|
||||
],
|
||||
bts=[BtsSettings(email="dev@example.org")],
|
||||
)
|
||||
|
||||
|
||||
class ConfigRoundTripTest(unittest.TestCase):
|
||||
def test_round_trip_preserves_all_accounts(self) -> None:
|
||||
config = sample_v2_config()
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
path = Path(tmp) / "weekly-activity.toml"
|
||||
save_config(config, path)
|
||||
self.assertEqual(load_config(path), config)
|
||||
|
||||
def test_save_writes_v2_version_header(self) -> None:
|
||||
text = format_toml(ActivityConfig(github=[GithubSettings(username="g")]))
|
||||
self.assertIn("version = 2", text)
|
||||
self.assertIn("[[github.accounts]]", text)
|
||||
|
||||
def test_save_creates_owner_only_file(self) -> None:
|
||||
config = ActivityConfig(bts=BtsSettings(email="x@example.org"))
|
||||
config = ActivityConfig(bts=[BtsSettings(email="x@example.org")])
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
path = Path(tmp) / "nested" / "cfg.toml"
|
||||
save_config(config, path)
|
||||
self.assertEqual(os.stat(path).st_mode & 0o777, 0o600)
|
||||
|
||||
def test_empty_config_emits_header_only(self) -> None:
|
||||
text = format_toml(ActivityConfig())
|
||||
self.assertEqual(load_config_from_text(text), ActivityConfig())
|
||||
self.assertNotIn("[[", text)
|
||||
|
||||
def test_empty_optionals_round_trip_to_defaults(self) -> None:
|
||||
config = ActivityConfig(
|
||||
github=GithubSettings(username="octocat"),
|
||||
bts=BtsSettings(email="a@b.c"),
|
||||
github=[GithubSettings(username="octocat")],
|
||||
bts=[BtsSettings(email="a@b.c")],
|
||||
)
|
||||
text = format_toml(config)
|
||||
reparsed = load_config_from_text(text)
|
||||
github = reparsed.github
|
||||
assert github is not None
|
||||
self.assertEqual(github.username, "octocat")
|
||||
self.assertEqual(github.token, "")
|
||||
self.assertIsNone(reparsed.launchpad)
|
||||
reparsed = load_config_from_text(format_toml(config))
|
||||
self.assertEqual(reparsed.github, [GithubSettings(username="octocat")])
|
||||
self.assertEqual(reparsed.bts, [BtsSettings(email="a@b.c")])
|
||||
self.assertEqual(reparsed.launchpad, [])
|
||||
|
||||
def test_escapes_quotes_backslashes_and_newlines(self) -> None:
|
||||
config = ActivityConfig(github=GithubSettings(username='a"b\\c\nd'))
|
||||
config = ActivityConfig(github=[GithubSettings(username='a"b\\c\nd')])
|
||||
reparsed = load_config_from_text(format_toml(config))
|
||||
github = reparsed.github
|
||||
assert github is not None
|
||||
self.assertEqual(github.username, 'a"b\\c\nd')
|
||||
self.assertEqual(reparsed.github[0].username, 'a"b\\c\nd')
|
||||
|
||||
def test_load_does_not_rewrite_file(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
path = Path(tmp) / "weekly-activity.toml"
|
||||
original = "[github]\nusername = 'octo'\n"
|
||||
path.write_text(original, encoding="utf-8")
|
||||
before = path.read_bytes()
|
||||
config = load_config(path)
|
||||
self.assertEqual(config.github, [GithubSettings(username="octo")])
|
||||
self.assertEqual(path.read_bytes(), before)
|
||||
|
||||
|
||||
class ConfigLoadingTest(unittest.TestCase):
|
||||
@@ -78,26 +117,123 @@ class ConfigLoadingTest(unittest.TestCase):
|
||||
|
||||
def test_unknown_section_rejected(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text("[gitea]\nusername = 'x'\n")
|
||||
load_config_from_text('version = 2\n\n[[gitea.accounts]]\nusername = "x"\n')
|
||||
|
||||
def test_unknown_key_rejected(self) -> None:
|
||||
def test_unknown_top_level_key_rejected(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text('[github]\nusername = "x"\ntokken = "t"\n')
|
||||
load_config_from_text('version = 2\nsources = 3\n')
|
||||
|
||||
def test_unknown_account_key_rejected(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text(
|
||||
'[github]\naccounts = [{username = "x", tokken = "t"}]\n'
|
||||
)
|
||||
|
||||
def test_section_must_hold_only_accounts_table(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text('[version]\nwrong = "shape"\n') # wrong type below
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text(
|
||||
'version = 2\n\n[github]\nusername = "legacy"\n' # v2 without .accounts
|
||||
)
|
||||
|
||||
def test_accounts_must_be_list_of_tables(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text('version = 2\n\n[github]\naccounts = "nope"\n')
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text('version = 2\n\n[gitlab]\naccounts = [42]\n')
|
||||
|
||||
def test_non_string_value_rejected(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text("[github]\nusername = 42\n")
|
||||
load_config_from_text(
|
||||
'version = 2\n\n[[github.accounts]]\nusername = 42\n'
|
||||
)
|
||||
|
||||
def test_bad_launchpad_mode_rejected(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text('[launchpad]\nusername = "u"\nmode = "oauth2"\n')
|
||||
load_config_from_text(
|
||||
'version = 2\n\n[[launchpad.accounts]]\nusername = "u"\nmode = "oauth2"\n'
|
||||
)
|
||||
|
||||
def test_bad_launchpad_service_rejected(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text(
|
||||
'version = 2\n\n[[launchpad.accounts]]\nusername = "u"\nservice = "qa"\n'
|
||||
)
|
||||
|
||||
def test_v2_section_without_accounts_table_rejected(self) -> None:
|
||||
# A version-tagged file must use the [[kind.accounts]] shape.
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text('version = 2\n\n[github]\nusername = "legacy"\n')
|
||||
|
||||
def test_account_errors_carry_position(self) -> None:
|
||||
text = (
|
||||
"version = 2\n"
|
||||
"\n[[launchpad.accounts]]\nusername = \"ok\"\n"
|
||||
"\n[[launchpad.accounts]]\nusername = \"bad\"\nservice = \"oops\"\n"
|
||||
)
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
path = Path(tmp) / "weekly-activity.toml"
|
||||
path.write_text(text, encoding="utf-8")
|
||||
with self.assertRaisesRegex(ConfigError, r"launchpad\.accounts\[1\]"):
|
||||
load_config(path)
|
||||
|
||||
def test_enabled_sources_stable_order(self) -> None:
|
||||
config = load_config_from_text(
|
||||
'[bts]\nemail = "e"\n\n[github]\nusername = "g"\n'
|
||||
config = ActivityConfig(
|
||||
bts=[BtsSettings(email="e")],
|
||||
github=[GithubSettings(username="g")],
|
||||
)
|
||||
self.assertEqual(config.enabled_sources(), ["github", "bts"])
|
||||
|
||||
|
||||
class ConfigMigrationTest(unittest.TestCase):
|
||||
V1_TEXT = """\
|
||||
[launchpad]
|
||||
username = "lp-user"
|
||||
mode = "credentials"
|
||||
credentials_file = "~/creds.json"
|
||||
|
||||
[github]
|
||||
username = "octo"
|
||||
token = "t0k"
|
||||
|
||||
[gitlab]
|
||||
url = "https://salsa.debian.org"
|
||||
username = "gl"
|
||||
token_env = "SALSA_TOKEN"
|
||||
|
||||
[bts]
|
||||
email = "dev@example.org"
|
||||
"""
|
||||
|
||||
def test_v1_file_migrates_to_single_unnamed_accounts_in_memory(self) -> None:
|
||||
config = load_config_from_text(self.V1_TEXT)
|
||||
self.assertEqual(config.enabled_sources(), ["launchpad", "github", "gitlab", "bts"])
|
||||
self.assertEqual(config.launchpad, [LaunchpadSettings(
|
||||
username="lp-user", mode="credentials", credentials_file="~/creds.json",
|
||||
)])
|
||||
self.assertEqual(config.github, [GithubSettings(username="octo", token="t0k")])
|
||||
self.assertEqual(config.gitlab, [GitlabSettings(
|
||||
url="https://salsa.debian.org", username="gl", token_env="SALSA_TOKEN",
|
||||
)])
|
||||
self.assertEqual(config.bts, [BtsSettings(email="dev@example.org")])
|
||||
|
||||
def test_saving_migrated_config_persists_v2_shape(self) -> None:
|
||||
config = load_config_from_text(self.V1_TEXT)
|
||||
text = format_toml(config)
|
||||
self.assertIn("version = 2", text)
|
||||
self.assertEqual(load_config_from_text(text), config)
|
||||
|
||||
def test_v1_unknown_keys_still_strict(self) -> None:
|
||||
with self.assertRaises(ConfigError):
|
||||
load_config_from_text('[github]\nusername = "u"\ntokken = "t"\n')
|
||||
|
||||
def test_v2_shaped_file_without_version_rejected(self) -> None:
|
||||
with self.assertRaisesRegex(ConfigError, "version"):
|
||||
load_config_from_text('[[github.accounts]]\nusername = "u"\n')
|
||||
|
||||
|
||||
class ConfigPathTest(unittest.TestCase):
|
||||
def test_default_path_uses_xdg_or_home(self) -> None:
|
||||
old = os.environ.get("XDG_CONFIG_HOME")
|
||||
try:
|
||||
@@ -110,13 +246,5 @@ class ConfigLoadingTest(unittest.TestCase):
|
||||
os.environ["XDG_CONFIG_HOME"] = old
|
||||
|
||||
|
||||
def load_config_from_text(text: str) -> ActivityConfig:
|
||||
"""Helper: parse TOML through the public loader without touching disk."""
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
path = Path(tmp) / "weekly-activity.toml"
|
||||
path.write_text(text, encoding="utf-8")
|
||||
return load_config(path)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
Reference in New Issue
Block a user