fix(tests): switch HMAC rejection tests from GET to POST endpoint - #89
Merged
Conversation
The four rejection tests in tests/integration/test_hmac.py (test_hmac_no_key_rejected, test_hmac_wrong_key_rejected, and their async variants) were calling client.get_enums(), which is a GET request. The Comlink HMAC service does not protect GET endpoints, so the server returned 200 OK regardless of authentication and the expected SwgohComlinkException was never raised, causing all four tests to fail with "DID NOT RAISE". Switch to client.get_player_arena(allycode=TEST_ALLYCODE, player_details_only=True), which is a POST request to /playerArena (an HMAC-protected endpoint). Use the shared TEST_ALLYCODE constant from conftest so the rejection tests align with the existing get_player and get_player_arena success tests. Verified end-to-end against a live HMAC-protected Comlink container: all four tests now correctly observe HTTP 403 HMACValidationError responses and pass; a positive control with valid keys returns a real playerDetailsOnly response. Fixes #88 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
MarTrepodi
added
bug
3 tasks
MarTrepodi
added a commit
that referenced
this pull request
May 11, 2026
**Draft release PR — promotes the current `develop` tip to `main`.** Once merged, dispatch the `comlink-python release` workflow from `main` to bump the version, regenerate `CHANGELOG.md`, tag, and publish to PyPI via OIDC. ## Contents — 6 commits since `v2.0.7` ``` 75feb67 fix(tests): switch HMAC rejection tests from GET to POST endpoint (#89) c9e3f59 fix(helpers): ensure arena payout time adjusts correctly when shifted to past e8a6e05 fix(helpers): handle multi-day offsets in get_arena_payout 51235c1 chore(dependabot): set target branch to `develop`, limit open PRs to 10 8c0aa3a ci(release): guard release workflow against non-main dispatch (#86) 523a2ca ci: align workflow triggers with develop branch model (#85) ``` ## What's in this release ### Bug fixes - **`get_arena_payout` returns past datetime when offset spans more than one day** ([#87](#87)) — `if` → `while` so payouts shifted into the past by 24+ hour offsets advance day-by-day until landing in the future. ### CI / infrastructure - **Workflow triggers aligned with the new `develop`-based branch model** ([#85](#85)) — `ci.yml`, `commitlint.yml`, and `integration.yml` now run on `[main, develop]` PRs and pushes; stale references to `1.0-maintenance` and `2.0-development` removed; redundant `test.yml` deleted. - **Release workflow hardened with a branch guard** ([#86](#86)) — both jobs in `release.yml` now require `github.ref == 'refs/heads/main'`, preventing accidental dispatch from `develop` or feature branches. - **HMAC integration tests fixed** ([#89](#89)) — rejection tests now call a POST endpoint (`/playerArena`) instead of `/enums` (GET, which the server doesn't HMAC-protect). ### Chore - **Dependabot configuration** ([commit `51235c1`](51235c1)) — `target-branch: develop` and `open-pull-requests-limit: 10` for the pip ecosystem. ## No user-facing API changes All changes are internal: bug fix in a helper, CI/test changes, dependabot config. No new methods, no signature changes, no removals. ## Release plan After this PR merges: 1. Trigger `comlink-python release` workflow from `main`: `gh workflow run release.yml --ref main`. 2. Workflow performs (gated by the new branch guard from #86): - `uvx hatch version minor` → `2.0.7` → `2.1.0` - `uvx git-changelog` regenerates `CHANGELOG.md` from commit history (Angular convention, semver) - Commit + tag (`v2.1.0`) + push - `uvx hatch build` (sdist + wheel) - Publish to PyPI via Trusted Publishing OIDC (`pypa/gh-action-pypi-publish` SHA-pinned at `v1.13.0`) 3. GitHub auto-creates a release from the tag. ## Test plan - [ ] All required CI checks pass on this PR (`Lint`, `Type Check`, `Test (Python 3.10..3.13)`, `Docs`, `Build`, `Validate Commit Messages`, `Integration Tests`). - [ ] Merge method: **rebase** (not squash) to preserve the individual PR commits on `main`'s linear history. - [ ] After merge, dispatch `release.yml` from `main` and verify: - Version bumps to `2.1.0` in `src/swgoh_comlink/version.py`. - `CHANGELOG.md` gets a new `## [v2.1.0]` section. - PyPI receives the new release. - GitHub Release is auto-created. ## Notes - This is a **draft PR** because the release cadence is your call — mark ready for review when you're prepared to cut `v2.1.0`. - Branch protection on `main` currently has `contexts: []` (no required checks). Consider applying the required-contexts list documented in `workflow-review-plan.md` once stable. Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #88
Summary
The four HMAC rejection tests in
tests/integration/test_hmac.py(test_hmac_no_key_rejected,test_hmac_wrong_key_rejected, and their async variants) were callingclient.get_enums(), which is a GET request to/enums. The Comlink HMAC service does not enforce HMAC on GET endpoints — only on POST — so the server returned 200 OK regardless of authentication and the expectedSwgohComlinkExceptionwas never raised. All four tests failed withFailed: DID NOT RAISE.This PR switches those tests to call
client.get_player_arena(allycode=TEST_ALLYCODE, player_details_only=True), a POST request to/playerArena(HMAC-protected). Uses the sharedTEST_ALLYCODEconstant fromconftest.pyso the rejection tests align with the existingget_playerandget_player_arenasuccess tests.Why this only surfaced now
Integration tests never ran on PRs targeting
developuntil #85 landed andintegration.ymlstarted triggering ondevelopPRs. PRs tomainhad been skipping these tests becausedevelopwas not in the trigger list, so the latent bug went unnoticed.Verification
Probed a live HMAC-protected Comlink container directly to confirm the diagnosis and exercised the new test code end-to-end:
GET /enumsPOST /playerArenaHMACValidationError(Authorization header missing)POST /playerArenaHMACValidationError(HMAC validation failed)test_key/test_secretPOST /playerArenaplayerDetailsOnlyresponseAll four updated tests
PASSagainst the live container.Test plan
Lint,Type Check,Test (Python 3.10..3.13),Docs,Build,Validate Commit Messages) remain green.tests/integration/test_hmac.py; client and server code untouched.Notes
test_hmac_*_request_succeedstests (which continue to pass) and by the positive-control valid-auth probe in this PR's verification.Generated with Claude Code