MIL-010: gym accounts from a credentials file #59

Merged
Tirsvad merged 3 commits from mil-010-credentials-file into main 2026-10-28 15:26:26 +01:00
12 changed files with 365 additions and 5 deletions
Showing only changes of commit 42f05bdd0f - Show all commits
+2
View File
@@ -92,6 +92,8 @@ $env:SNACK_AND_LIFT_EMAIL = "yourname@test.com"
$env:SNACK_AND_LIFT_PASSWORD = "your password"
```
Instead of setting variables by hand, you can put the same lines in a file named `.env.gym` in the project folder (`SNACK_AND_LIFT_EMAIL=...` and `SNACK_AND_LIFT_PASSWORD=...`, one per line). The bot reads it at start; `--env-file PATH` names another file, and a real environment variable wins over the file. The file `.env.gym` is ignored by git: never commit it. Two optional lines, `SNACK_AND_LIFT_ADMIN_EMAIL` and `SNACK_AND_LIFT_ADMIN_PASSWORD`, hold the admin account of the practice site; the bot loads them but does not use them yet.
### Start the bot
With the environment active:
@@ -0,0 +1,61 @@
# SQA Review Record
## Metadata
| Key | Value |
| --- | --- |
| ID | RC-026 |
| CrossReference | [MIL-010], [QC-PY-001] |
## Version History
| Date | Status | Author | Reviewer | Change | Commit |
| --- | --- | --- | --- | --- | --- |
| 2026-10-28 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S02 | Review record, full review of the source and tests of MIL-010 | pending |
---
## Artifact Under Review
- Instance reviewed: [MIL-010]
- Checklist used: [QC-PY-001] (`QC-PY-001`)
- Scope: full review of the source and tests of MIL-010
- Language and domain: n/a
- Language reviewer: none
- Mode: autonomous, not independent (the reviewer is `AI as stand-in for S02`)
## Checklist Results
| # | Criterion | Status | Evidence/Notes |
| --- | --- | --- | --- |
| 1 | Packages, modules, functions, variables, classes and constants follow PEP 8 casing (`snake_case`, `PascalCase`, `UPPER_SNAKE`) | Pass | Modules, functions and variables are `snake_case`, classes `PascalCase`, constants `UPPER_SNAKE`; `ruff check` with the N rules passes. |
| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names state purpose (for example `retry`, `login`, `book_class`, `get_my_bookings`); no single-letter names outside comprehensions. |
| 3 | Code is produced by the project's formatter and passes its linter with no unexplained suppressions | Pass | `ruff format` leaves 0 files changed and `ruff check` reports 0 findings; there is no suppression. |
| 4 | Every function and method signature is type-annotated, including `-> None` | Pass | Every function and method has annotations (ruff ANN rules pass), including `-> None`. |
| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) | Pass | No bare `except`; specific exceptions are caught, and re-raised with `raise ... from` where the cause matters. |
| 6 | No mutable default arguments and no shadowed builtins | Pass | No mutable default arguments; no builtin is shadowed (ruff B and A-style checks pass). |
| 7 | Files, locks and connections are managed with context managers | Pass | Files and the browser are managed with `with`/`try-finally`; no unmanaged file handle exists. |
| 8 | Public modules, classes and functions have docstrings that say what, not how | Pass | Public modules, classes and functions have Doxygen docstrings (`@brief`, `@param`, `@return`). |
| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output | Pass | No print calls; messages go through `logging`. |
| 10 | Classes and operations trace to the Design Class Diagram they implement; deviations are recorded | N-A | No Design Class Diagram exists; ADR-0001 records that design artifacts are skipped. Classes follow ADR-0002. |
| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network | Pass | 273 unit tests need no browser or network (new: the admin account, half-set variables, the env file parser, precedence of the environment, `--env-file`); an autouse fixture keeps every CLI test in `tmp_path`, so no test can read `.env` or `.env.gym`; 21 container tests pass; coverage of the non-infrastructure modules is 100%. |
| 12 | Type checker runs in strict mode without errors; `Any` is justified in a comment | N-A | No strict type checker is configured; annotations are complete and checked by ruff. |
| 13 | Dependencies are declared and pinned in the project's dependency file, none unused | Pass | `pyproject.toml` declares Selenium (runtime) and pytest, pytest-cov, ruff (dev) with lower bounds; none is unused. |
| 14 | Each piece of knowledge (a business rule, constant, format, validation or query) is defined in one place; code is merged only where it expresses the same knowledge, not where it merely looks alike | Pass | Each rule, constant and message exists once (constants in `constants.py`, rules in `domain/`). |
| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles | Pass | `domain` and `application` import no Selenium; only `infrastructure` imports it; no import cycle exists. |
| 16 | SOLID holds: a class or module has one reason to change; new behaviour is added by extension, not by editing a type test in several places; subclasses honour the contract of their base; protocols are narrow; high-level code depends on protocols or abstract types and receives its implementations | Pass | One reason to change per module; the adapter hides Selenium behind a port; fakes honour the port contract. |
| 17 | Source is laid out in the layers and folders of the clean architecture folder structure (`domain`, `application`, `infrastructure`, `presentation` packages under `src/`, and `unit`, `integration` and `e2e` under `tests/`), in `snake_case`, and imports between the layers point inward | Pass | Layout follows `src/snack_and_lift/{domain,application,infrastructure}` and `tests/{unit,integration,e2e}`; there is no presentation layer for a command-line bot (ADR-0002). |
| 18 | File and folder names are English, contain no space and no character outside letters, digits, `-`, `_` and `.`, and follow the casing of the language for files | Pass | All file and folder names are English lowercase with `_`, `-` and `.` only. |
## Overall Verdict
Go — the code of MIL-010 meets every Mandatory criterion of QC-PY-001. No blocking finding. The review is not independent: the AI wrote and reviewed it as stand-in for S02, and S01 can challenge it in the end-of-run summary.
## Action Items
| Action | Owner | Due |
| --- | --- | --- |
| None | S01 | - |
---
[MIL-010]: ../../milestones/mil-010-credentials-file.md
[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md
+4
View File
@@ -39,6 +39,10 @@ $env:SNACK_AND_LIFT_EMAIL = "yourname@test.com"
$env:SNACK_AND_LIFT_PASSWORD = "your password"
```
## Kontofilen
I stedet for at sætte variabler i hånden kan du skrive de samme linjer i en fil `.env.gym` i projektmappen (`SNACK_AND_LIFT_EMAIL=...` og `SNACK_AND_LIFT_PASSWORD=...`). Botten læser den ved start; `--env-file PATH` peger på en anden fil, og en rigtig miljøvariabel vinder over filen. Git ignorerer `.env.gym`: læg den aldrig i repositoriet. De valgfrie `SNACK_AND_LIFT_ADMIN_EMAIL` og `SNACK_AND_LIFT_ADMIN_PASSWORD` gemmer øvelsessidens admin-konto; botten indlæser dem, men bruger dem ikke endnu.
## Kør og test
```bash
+4
View File
@@ -39,6 +39,10 @@ $env:SNACK_AND_LIFT_EMAIL = "yourname@test.com"
$env:SNACK_AND_LIFT_PASSWORD = "your password"
```
## Fail akaun
Daripada menetapkan pemboleh ubah secara manual, letakkan baris yang sama dalam fail `.env.gym` di folder projek (`SNACK_AND_LIFT_EMAIL=...` dan `SNACK_AND_LIFT_PASSWORD=...`). Bot membacanya semasa bermula; `--env-file PATH` menamakan fail lain, dan pemboleh ubah persekitaran sebenar mengatasi fail. Git mengabaikan `.env.gym`: jangan sekali-kali commit. `SNACK_AND_LIFT_ADMIN_EMAIL` dan `SNACK_AND_LIFT_ADMIN_PASSWORD` yang pilihan menyimpan akaun admin laman latihan; bot memuatkannya tetapi belum menggunakannya.
## Jalankan dan uji
```bash
+4
View File
@@ -39,6 +39,10 @@ $env:SNACK_AND_LIFT_EMAIL = "yourname@test.com"
$env:SNACK_AND_LIFT_PASSWORD = "your password"
```
## The account file
Instead of setting variables by hand, put the same lines in a file `.env.gym` in the project folder (`SNACK_AND_LIFT_EMAIL=...` and `SNACK_AND_LIFT_PASSWORD=...`). The bot reads it at start; `--env-file PATH` names another file, and a real environment variable wins over the file. Git ignores `.env.gym`: never commit it. The optional `SNACK_AND_LIFT_ADMIN_EMAIL` and `SNACK_AND_LIFT_ADMIN_PASSWORD` hold the practice site's admin account; the bot loads them but does not use them yet.
## Run and test
```bash
+29 -1
View File
@@ -30,6 +30,34 @@ def load_credentials(environment: Mapping[str, str]) -> Credentials:
if not email or not password:
raise MissingCredentialsError(
f"Set {constants.ACCOUNT_EMAIL_ENV} and {constants.ACCOUNT_PASSWORD_ENV} "
"in the environment (they must not be empty)"
"in the environment or the env file (they must not be empty)"
)
return Credentials(email=email, password=password)
def load_admin_credentials(environment: Mapping[str, str]) -> Credentials | None:
"""@brief Read the optional admin account from the environment.
@param environment The environment variables, for example `os.environ`.
@return The credentials, or None when neither variable is set.
@exception MissingCredentialsError Only one of the two variables is set; the message
names the missing variable and never a value.
"""
email = environment.get(constants.ADMIN_EMAIL_ENV, "").strip()
password = environment.get(constants.ADMIN_PASSWORD_ENV, "")
if not email and not password:
return None
if not email:
raise MissingCredentialsError(_admin_missing(constants.ADMIN_EMAIL_ENV))
if not password:
raise MissingCredentialsError(_admin_missing(constants.ADMIN_PASSWORD_ENV))
return Credentials(email=email, password=password)
def _admin_missing(name: str) -> str:
"""@brief Build the message for a half-set admin account.
@param name The variable that is missing or empty.
@return The message; it names the variable only.
"""
return constants.MESSAGE_ADMIN_HALF_SET.format(name=name)
+21 -3
View File
@@ -12,7 +12,11 @@ from pathlib import Path
from snack_and_lift import constants
from snack_and_lift.application.booking import book_next_tuesday, book_target_classes
from snack_and_lift.application.credentials import Credentials, load_credentials
from snack_and_lift.application.credentials import (
Credentials,
load_admin_credentials,
load_credentials,
)
from snack_and_lift.application.ports import GymPort
from snack_and_lift.application.retry import Sleep
from snack_and_lift.application.session import login_with_retry
@@ -23,6 +27,7 @@ from snack_and_lift.domain.report import format_detail_list, format_summary
from snack_and_lift.domain.verification import format_verification
from snack_and_lift.infrastructure.browser import create_driver, default_profile_dir
from snack_and_lift.infrastructure.console_recorder import ConsoleRecorder
from snack_and_lift.infrastructure.env_file import read_env_file
from snack_and_lift.infrastructure.selenium_gym import SeleniumGym
## Builds the gym site from (headless, profile folder, keep open).
@@ -69,7 +74,8 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace:
description="Books the Tuesday and Thursday 6pm classes at the Snack and Lift gym.",
epilog=(
"exit codes: 0 done, 1 a step failed (also when every retry failed), "
"2 credentials missing, 3 the My Bookings page does not match the bookings made"
"2 credentials missing in the environment or the env file, "
"3 the My Bookings page does not match the bookings made"
),
)
parser.add_argument(
@@ -110,6 +116,16 @@ def parse_args(argv: Sequence[str] | None = None) -> argparse.Namespace:
f"drops a request (default: {constants.RETRY_COUNT}, at least 1)"
),
)
parser.add_argument(
"--env-file",
type=Path,
default=Path(constants.ENV_FILE_NAME),
metavar="PATH",
help=(
"file with the gym accounts as KEY=VALUE lines "
f"(default: ./{constants.ENV_FILE_NAME}); a real environment variable wins"
),
)
args = parser.parse_args(argv)
if args.keep_open is None:
args.keep_open = not args.headless
@@ -242,7 +258,9 @@ def _main(
_use_utf8_output()
args = parse_args(argv)
try:
credentials = load_credentials(os.environ)
environment = {**read_env_file(args.env_file), **os.environ}
credentials = load_credentials(environment)
load_admin_credentials(environment) # load-only for now; nothing uses it yet
except MissingCredentialsError as error:
logger.error("%s", error)
return 2
+13
View File
@@ -70,6 +70,19 @@ ACCOUNT_EMAIL_ENV: Final[str] = "SNACK_AND_LIFT_EMAIL"
## Name of the environment variable that holds the account password.
ACCOUNT_PASSWORD_ENV: Final[str] = "SNACK_AND_LIFT_PASSWORD"
## Names of the environment variables that hold the admin account (read, not used yet).
ADMIN_EMAIL_ENV: Final[str] = "SNACK_AND_LIFT_ADMIN_EMAIL"
ADMIN_PASSWORD_ENV: Final[str] = "SNACK_AND_LIFT_ADMIN_PASSWORD"
## Error for an admin account with one variable only; {name} is the missing variable.
MESSAGE_ADMIN_HALF_SET: Final[str] = (
"Set {name} too: the admin account needs both "
"SNACK_AND_LIFT_ADMIN_EMAIL and SNACK_AND_LIFT_ADMIN_PASSWORD"
)
## Default file with the gym accounts, relative to the current directory.
ENV_FILE_NAME: Final[str] = ".env.gym"
## Folder where screenshots of a container run are written.
SCREENSHOT_DIR: Final[str] = "docs/images"
@@ -0,0 +1,44 @@
"""@file env_file.py
@brief Reads `KEY=VALUE` lines from an env file (never logs a value).
"""
from pathlib import Path
## Prefix a line may carry so the file can also be sourced by a shell.
_EXPORT_PREFIX = "export "
## Quote characters that may surround a value.
_QUOTES = ("'", '"')
def _unquote(value: str) -> str:
"""@brief Strip one pair of matching surrounding quotes.
@param value The text after the `=`.
@return The value without the surrounding pair, otherwise unchanged.
"""
if len(value) >= 2 and value[0] == value[-1] and value[0] in _QUOTES:
return value[1:-1]
return value
def read_env_file(path: Path) -> dict[str, str]:
"""@brief Read the variables of an env file.
Blank lines, `#` comments, lines without `=` and an empty key are skipped.
@param path The file.
@return The variables; empty when the file does not exist.
"""
if not path.is_file():
return {}
variables: dict[str, str] = {}
for line in path.read_text(encoding="utf-8").splitlines():
stripped = line.strip()
if not stripped or stripped.startswith("#") or "=" not in stripped:
continue
key, _, value = stripped.partition("=")
key = key.strip().removeprefix(_EXPORT_PREFIX).strip()
if key:
variables[key] = _unquote(value.strip())
return variables
+71
View File
@@ -18,6 +18,12 @@ DUMMY_EMAIL = "dummy@example.invalid"
DUMMY_PASSWORD = "dummy-password"
@pytest.fixture(autouse=True)
def isolated_working_directory(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
"""Run from an empty folder so the default env file is never a real one."""
monkeypatch.chdir(tmp_path)
@pytest.fixture
def credentials_env(monkeypatch: pytest.MonkeyPatch) -> None:
monkeypatch.setenv(constants.ACCOUNT_EMAIL_ENV, DUMMY_EMAIL)
@@ -324,3 +330,68 @@ def test_main_passes_options_to_the_factory(tmp_path: Path) -> None:
main(["--headless", "--profile-dir", str(tmp_path)], factory)
assert received == [(True, tmp_path, False)]
def _write_env_file(path: Path, email: str, password: str) -> Path:
path.write_text(
f"{constants.ACCOUNT_EMAIL_ENV}={email}\n"
f"export {constants.ACCOUNT_PASSWORD_ENV}='{password}'\n",
encoding="utf-8",
)
return path
def test_env_file_defaults_to_the_gym_file_name() -> None:
assert parse_args([]).env_file == Path(constants.ENV_FILE_NAME)
def test_env_file_option_is_read(tmp_path: Path) -> None:
assert parse_args(["--env-file", str(tmp_path / "x.env")]).env_file == tmp_path / "x.env"
def test_main_reads_the_account_from_the_env_file(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
monkeypatch.delenv(constants.ACCOUNT_EMAIL_ENV, raising=False)
monkeypatch.delenv(constants.ACCOUNT_PASSWORD_ENV, raising=False)
env_file = _write_env_file(tmp_path / "accounts.env", "file@example.invalid", "file-pass")
gym = FakeGym()
code = main(["--env-file", str(env_file)], lambda *_: gym)
assert code == 0
assert gym.logins == [("file@example.invalid", "file-pass")]
def test_main_lets_a_real_environment_variable_win_over_the_env_file(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
monkeypatch.setenv(constants.ACCOUNT_EMAIL_ENV, DUMMY_EMAIL)
monkeypatch.setenv(constants.ACCOUNT_PASSWORD_ENV, DUMMY_PASSWORD)
env_file = _write_env_file(tmp_path / "accounts.env", "file@example.invalid", "file-pass")
gym = FakeGym()
code = main(["--env-file", str(env_file)], lambda *_: gym)
assert code == 0
assert gym.logins == [(DUMMY_EMAIL, DUMMY_PASSWORD)]
def test_main_exits_2_when_neither_environment_nor_file_has_the_account(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
monkeypatch.delenv(constants.ACCOUNT_EMAIL_ENV, raising=False)
monkeypatch.delenv(constants.ACCOUNT_PASSWORD_ENV, raising=False)
assert main(["--env-file", str(tmp_path / "absent.env")], lambda *_: FakeGym()) == 2
def test_main_exits_2_when_the_admin_account_is_half_set(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
monkeypatch.setenv(constants.ACCOUNT_EMAIL_ENV, DUMMY_EMAIL)
monkeypatch.setenv(constants.ACCOUNT_PASSWORD_ENV, DUMMY_PASSWORD)
monkeypatch.setenv(constants.ADMIN_EMAIL_ENV, "admin@example.invalid")
monkeypatch.delenv(constants.ADMIN_PASSWORD_ENV, raising=False)
assert main(["--env-file", str(tmp_path / "absent.env")], lambda *_: FakeGym()) == 2
+53 -1
View File
@@ -5,7 +5,11 @@
import pytest
from snack_and_lift import constants
from snack_and_lift.application.credentials import Credentials, load_credentials
from snack_and_lift.application.credentials import (
Credentials,
load_admin_credentials,
load_credentials,
)
from snack_and_lift.domain.errors import MissingCredentialsError
DUMMY_PASSWORD = "dummy-secret-value"
@@ -42,3 +46,51 @@ def test_load_credentials_rejects_missing_or_empty(environment: dict[str, str])
def test_repr_hides_the_password() -> None:
assert DUMMY_PASSWORD not in repr(Credentials("dummy@example.invalid", DUMMY_PASSWORD))
def test_load_admin_credentials_reads_both_variables() -> None:
environment = {
constants.ADMIN_EMAIL_ENV: " admin@example.invalid ",
constants.ADMIN_PASSWORD_ENV: DUMMY_PASSWORD,
}
assert load_admin_credentials(environment) == Credentials(
"admin@example.invalid", DUMMY_PASSWORD
)
@pytest.mark.parametrize(
"environment",
[
{},
{constants.ADMIN_EMAIL_ENV: "", constants.ADMIN_PASSWORD_ENV: ""},
{constants.ADMIN_EMAIL_ENV: " "},
],
)
def test_load_admin_credentials_is_none_when_not_set(environment: dict[str, str]) -> None:
assert load_admin_credentials(environment) is None
@pytest.mark.parametrize(
("environment", "missing"),
[
({constants.ADMIN_EMAIL_ENV: "admin@example.invalid"}, constants.ADMIN_PASSWORD_ENV),
(
{constants.ADMIN_EMAIL_ENV: "admin@example.invalid", constants.ADMIN_PASSWORD_ENV: ""},
constants.ADMIN_PASSWORD_ENV,
),
({constants.ADMIN_PASSWORD_ENV: DUMMY_PASSWORD}, constants.ADMIN_EMAIL_ENV),
(
{constants.ADMIN_EMAIL_ENV: "", constants.ADMIN_PASSWORD_ENV: DUMMY_PASSWORD},
constants.ADMIN_EMAIL_ENV,
),
],
)
def test_load_admin_credentials_rejects_half_set(environment: dict[str, str], missing: str) -> None:
with pytest.raises(MissingCredentialsError) as caught:
load_admin_credentials(environment)
message = str(caught.value)
assert message.startswith(f"Set {missing} too")
assert DUMMY_PASSWORD not in message
assert "admin@example.invalid" not in message
+59
View File
@@ -0,0 +1,59 @@
"""@file test_env_file.py
@brief The env file reader parses KEY=VALUE lines (dummy values in tmp_path only).
"""
from pathlib import Path
from snack_and_lift.infrastructure.env_file import read_env_file
def _read(tmp_path: Path, text: str) -> dict[str, str]:
path = tmp_path / "accounts.env"
path.write_text(text, encoding="utf-8")
return read_env_file(path)
def test_read_env_file_returns_empty_when_file_is_missing(tmp_path: Path) -> None:
assert read_env_file(tmp_path / "absent.env") == {}
def test_read_env_file_reads_pairs(tmp_path: Path) -> None:
assert _read(tmp_path, "A=1\nB=two\n") == {"A": "1", "B": "two"}
def test_read_env_file_skips_blank_lines_comments_and_lines_without_equals(
tmp_path: Path,
) -> None:
text = "\n# comment\n # indented comment\nnot a pair\nA=1\n"
assert _read(tmp_path, text) == {"A": "1"}
def test_read_env_file_accepts_export_prefix(tmp_path: Path) -> None:
assert _read(tmp_path, "export A=1\n") == {"A": "1"}
def test_read_env_file_strips_whitespace_around_key(tmp_path: Path) -> None:
assert _read(tmp_path, " A =1\n") == {"A": "1"}
def test_read_env_file_strips_one_pair_of_quotes(tmp_path: Path) -> None:
text = "A=\"dummy one\"\nB='dummy two'\nC=\"'both'\"\n"
assert _read(tmp_path, text) == {"A": "dummy one", "B": "dummy two", "C": "'both'"}
def test_read_env_file_keeps_unmatched_quotes_and_lone_quote(tmp_path: Path) -> None:
assert _read(tmp_path, 'A="dummy\'\nB="\n') == {"A": "\"dummy'", "B": '"'}
def test_read_env_file_keeps_equals_and_hash_inside_value(tmp_path: Path) -> None:
assert _read(tmp_path, "A=x=y#z\n") == {"A": "x=y#z"}
def test_read_env_file_ignores_empty_key(tmp_path: Path) -> None:
assert _read(tmp_path, "=value\n") == {}
def test_read_env_file_keeps_empty_value(tmp_path: Path) -> None:
assert _read(tmp_path, "A=\n") == {"A": ""}