Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ An integration that outgrows one of these files can turn it into a package of th

**Sandboxed local runs** by pointing `SMORG_CONFIG_DIR` at a scratch directory and setting `SMORG_CREDENTIAL_STORE=file` to run against it instead of the OS Keychain.

**Experimental integrations.** A manifest that sets `experimental=True` is registered only when `SMORG_EXPERIMENTAL` names it, comma-separated: `SMORG_EXPERIMENTAL=gcal smorg`. That lets a release carry an integration that is still being built without offering its tab; anyone else gets the same "not supported" it would get for an unregistered id. Drop the flag when the integration ships.

## What is expected: code quality, comment quality, test quality

I don't mind slop code (a lot of the core and auth were made with AI). But enforce the following:
Expand Down
2 changes: 2 additions & 0 deletions src/smorg/core/contract.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ class Manifest:
connections: tuple[AuthPath, ...]
stale_after: timedelta
actions: tuple[Action, ...]
# Registered only when SMORG_EXPERIMENTAL names it: a release carries the code, not the tab.
experimental: bool = False

def __post_init__(self) -> None:
keys = [action.key for action in self.actions]
Expand Down
26 changes: 24 additions & 2 deletions src/smorg/core/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,44 @@

from __future__ import annotations

import os

from smorg.core.contract import Integration, Manifest

EXPERIMENTAL_ENV = "SMORG_EXPERIMENTAL"


class UnknownIntegration(Exception):
"""No integration by that id is registered in this build."""


def _opted_in() -> frozenset[str]:
"""The integration ids SMORG_EXPERIMENTAL names, comma-separated: "gcal" or "gcal,slack"."""
setting = os.environ.get(EXPERIMENTAL_ENV, "")
opted: set[str] = set()
for name in setting.split(","):
trimmed = name.strip()
if trimmed:
opted.add(trimmed)
return frozenset(opted)


def _by_id() -> dict[str, Integration]:
# Imported per call, not at module load, so tests can swap the allowlist without reloading.
from smorg import integrations

opted_in = _opted_in()
registry: dict[str, Integration] = {}
seen: set[str] = set()
for entry in integrations.INTEGRATIONS:
identifier = entry.manifest.id
if identifier in registry:
manifest = entry.manifest
identifier = manifest.id
if identifier in seen:
raise ValueError(f"two registered integrations share id {identifier!r}")
seen.add(identifier)
hidden = manifest.experimental and identifier not in opted_in
if hidden:
continue
registry[identifier] = entry
return registry

Expand Down
1 change: 1 addition & 0 deletions src/smorg/integrations/gcal/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
),
Action(id="today", label="Jump to today", key="t", action_class=ActionClass.LOCAL),
),
experimental=True,
)


Expand Down
17 changes: 17 additions & 0 deletions tests/core/test_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
Unavailable,
)
from smorg.core.registry import (
EXPERIMENTAL_ENV,
UnknownIntegration,
get_integration,
known_integration_ids,
Expand All @@ -39,13 +40,15 @@ def manifest(
identifier: str = "fake",
actions: tuple[Action, ...] = (),
connections: tuple[AuthPath, ...] = DEFAULT_CONNECTIONS,
experimental: bool = False,
) -> Manifest:
return Manifest(
id=identifier,
display_name=identifier.title(),
connections=connections,
stale_after=timedelta(minutes=5),
actions=actions,
experimental=experimental,
)


Expand Down Expand Up @@ -158,6 +161,20 @@ def test_registry_refuses_two_integrations_sharing_an_id(registered):
get_integration("linear")


def test_an_experimental_integration_is_hidden_until_the_env_var_names_it(registered, monkeypatch):
registered(manifest("linear"), manifest("gcal", experimental=True))
monkeypatch.delenv(EXPERIMENTAL_ENV, raising=False)

assert known_integration_ids() == ("linear",)
with pytest.raises(UnknownIntegration):
get_integration("gcal")

monkeypatch.setenv(EXPERIMENTAL_ENV, "gcal")

assert known_integration_ids() == ("gcal", "linear")
assert get_integration("gcal").manifest.id == "gcal"


def test_manifests_enumerates_every_registered_one_sorted_by_id(registered):
"""Also proves manifests() reads the same per-call INTEGRATIONS import as
_by_id: the registered fixture only takes effect through that indirection.
Expand Down
2 changes: 1 addition & 1 deletion tests/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def test_version_flag_omits_dev_on_a_release_build(monkeypatch, capsys):


def test_the_allowlist_is_what_this_build_registers():
assert known_integration_ids() == ("gcal", "github", "linear", "spotify")
assert known_integration_ids() == ("github", "linear", "spotify")


def test_connect_rejects_an_unknown_integration(capsys):
Expand Down
6 changes: 4 additions & 2 deletions tests/test_seams.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import re
from pathlib import Path

from smorg.core.registry import manifests
from smorg import integrations

# Anchored to this file, not the working directory: a relative path finds nothing when pytest
# runs from anywhere else, and every check below then passes having read no source at all.
Expand Down Expand Up @@ -40,8 +40,10 @@ def _package_source(integration_id: str) -> str:


def test_every_declared_action_key_is_bound():
# The allowlist, not the registry: a hidden integration's keys still have to be bound.
offenders: list[str] = []
for manifest in manifests():
for entry in integrations.INTEGRATIONS:
manifest = entry.manifest
source = _package_source(manifest.id)
for action in manifest.actions:
binding = f'Binding("{action.key}"'
Expand Down