Skip to content

test: Add tests for handle commands#64

Draft
LarsV123 wants to merge 12 commits into
mainfrom
larsv/2026/05/06-handle
Draft

test: Add tests for handle commands#64
LarsV123 wants to merge 12 commits into
mainfrom
larsv/2026/05/06-handle

Conversation

@LarsV123

@LarsV123 LarsV123 commented May 6, 2026

Copy link
Copy Markdown
Contributor

No description provided.

LarsV123 added 10 commits May 6, 2026 11:13
Add pytest-cov and configure pyproject.toml to fail the test run if
coverage drops below the current baseline (44.51% across commands and
cli, displayed as 45%). The 44% floor leaves about half a percent of
headroom so trivial refactors do not flip the build red. Ratchet the
threshold up as the per-service refactor PRs add tests.

Note that test files are currently included in the coverage
denominator. Excluding them via [tool.coverage.run] omit would lower
the displayed number to ~38% but give a more meaningful 'production
code with tests' metric. Worth doing once the per-service work
stabilizes.
…ession

LambdaService held only a profile and a lambda client built in __init__,
no real state. Convert the four methods to free functions taking
boto3.Session, in line with the established pattern. Update
commands/awslambda.py to pass ctx.session.

Two small cleanups landed in the same commit because they touch the
same function bodies:
- Renamed delete_old_versions to cleanup_old_versions to free up the
  name for the click command and to describe the dry-run behavior
  more honestly.
- Use FunctionName (instead of FunctionArn) when calling list_aliases
  and list_versions_by_function. Both AWS APIs accept either, but the
  name is the more idiomatic choice and makes the code testable
  against moto's stricter handling.
Three behaviors covered:
- delete-old-versions: real moto Lambda function with three published
  versions and an alias on v2; assert that only $LATEST and the
  aliased version remain.
- concurrency: real moto Lambda functions with put_function_concurrency;
  assert the JSON file written by the command is sorted descending by
  reserved concurrency, with un-reserved functions last.
- invoke: stubbed lambda client because moto stores async invocations
  internally without exposing them through any public API. The stub
  lets us verify that FunctionName and Payload are forwarded correctly.

Helpers (_create_function, _publish_versions, _create_alias,
_set_reserved_concurrency, _list_versions, _zip_handler) keep each
test body to about ten lines. They are intentionally co-located with
the tests rather than promoted to conftest until a second test module
needs them.

Adds moto[awslambda-simple] to dev deps. The 'simple' variant supports
all metadata operations (create, publish_version, alias, concurrency)
without needing Docker.
The CI build runs 'ruff format --check' and fails if any file needs
reformatting. Without a pre-commit hook, contributors discover this
only after pushing, which has been a recurring source of fixup commits.

Use the local-via-uv flavor so the hook runs the same ruff version
that pyproject.toml pins for everything else (CI, manual invocation).
This avoids a second source of truth for the ruff version.

Hooks are installed per-clone via 'uvx pre-commit install'; documented
in the README.
SwsService held genuine state (cached OAuth token), so the dataclass
shape from ApiClient applies. Differences from ApiClient kept it
separate rather than generalizing:
- Different Cognito domain (sws-auth.auth.eu-west-1...)
- Different secret name (SearchInfrastructureCredentials)
- Different secret schema (username/password instead of
  backendClientId/backendClientSecret)
- Endpoint pair selected by checking for 'prod' in profile name

Take session in the constructor instead of profile, and lazy-load
credentials via cached_property. _token is init=False so the cache
cannot be set through the constructor. Move get_mappings to a free
function taking the client.

The endpoint constants moved to module level so tests can import them.
Two behaviors covered, using moto for Secrets Manager and the
responses library for the OAuth token POST and the SWS API GET:
- happy path: returns the index mapping JSON, asserts a known nested
  field is present.
- 404 path: SWS returns 404 for the index, command exits 1 with an
  error message on stderr.

A third test for prod-vs-non-prod endpoint detection was dropped
because boto3.Session(profile_name='nva-prod') resolves the profile
against the developer's ~/.aws/config before moto can intercept,
making the test depend on the dev's local AWS configuration. The
prod logic is one line and easy to read; can revisit if it ever
gets more complex.
Three services in this command map to three different shapes:

HandleApiService -> free functions taking ApiClient. Was a duplicate
of the standard NVA-API token-cached wrapper, so it now consumes the
shared ApiClient instead of recreating its own SSM/Secrets/Cognito
plumbing. Public surface: create_handle, update_handle.

HandleTaskWriterService -> single free function build_task. The class
held no state (only two prefix constants) and exposed one public
method. Constants moved to module level.

HandleTaskExecutorService -> stays as a class. It owns a SQLite
connection, an action switcher dict, and an instance of
PublicationApiService that has not been converted yet. Constructor
now takes session + profile + folder; it builds an ApiClient
internally and bridges to PublicationApiService(profile) until that
service is converted in a later PR.
Two behaviors covered against a moto DynamoDB table that matches the
table-name regex production code expects:
- prepare writes one task per publication with the correct action
  derived from build_task (a publication with a Sikt-managed top
  handle gets 'nop'; one without any handles gets 'create_new_top').
- prepare only processes publications whose PK0 matches the requested
  customer + resource owner, ignoring other rows in the same table.

Publications are seeded in the same compressed format the production
code expects: zlib-deflated JSON, base64-encoded, stored in the
'data' attribute. The seeding helpers handle that.

The handle execute command is not tested here. Its eager construction
of PublicationApiService still hits SSM/Secrets/Cognito at startup,
which we cannot easily mock until that service is converted to the
ApiClient shape (later PR).
@LarsV123 LarsV123 changed the title Larsv/2026/05/06 handle test: Add tests for handle commands May 8, 2026
LarsV123 added 2 commits May 8, 2026 15:11
HandleTaskExecutorService used to take both session (for the
ApiClient-backed handle_api) and profile (for the still-profile-based
PublicationApiService). The dual constructor leaked the in-progress
state of the wider refactor into the call site.

Move the bridge inside the executor: take only profile, build a session
internally for ApiClient via build_session(profile). When
PublicationApiService is converted in a later PR, the build_session
line goes away and the executor will take a session directly.
@LarsV123 LarsV123 requested a review from torbjokv May 8, 2026 13:14
@LarsV123 LarsV123 marked this pull request as ready for review May 8, 2026 13:14
@torbjokv

torbjokv commented May 8, 2026

Copy link
Copy Markdown
Contributor

I'm replacing some of this stuff here #67 so suggest to skip this PR

@LarsV123 LarsV123 marked this pull request as draft May 8, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants