Refresh the schema tooling and add/improve test suite - #45
Merged
Conversation
Move the schema artifacts out of the repo root: - scripts/dump_schema.py writes the fixture via a __file__-relative path (run with make schema) - tests/fixtures/schema.json, refreshed from the live introspection endpoint - remove the stale schema-20230225.txt Add tests/test_schema.py, which derives the GraphQL types the library's queries use (input types from birdbuddy.queries, object types from the SightingType enum) and asserts the committed schema still defines them, so drift surfaces automatically with no hand-maintained lists. Two input types the guard would otherwise flag are exempted via a documented _KNOWN_MISSING allowlist: the API dropped SightingCreateInput and SightingCreateCheckProgressInput when the generic sightingCreate flow was removed (superseded by postcardCollect), so sighting_create and sighting_create_check_progress are non-functional but kept for backwards compatibility. See jhansche#29 (schema drift).
jhansche
reviewed
Jul 22, 2026
There was a problem hiding this comment.
⚠️ Not ready to approve
The new payload sanitizer and committed JSON fixture still include an embedded decoded report-token blob that can contain sensitive identifiers and should be redacted.
Pull request overview
This PR modernizes schema/fixture tooling and significantly expands/reorganizes the test suite to better guard against Bird Buddy GraphQL schema drift and regressions in model/client logic. It also adds helper scripts for refreshing the committed schema and capturing sanitized real-world API payloads to seed future fixtures.
Changes:
- Add schema drift guard tests and refresh the committed GraphQL schema fixture.
- Add a payload-dump + sanitizer script (dotenv-driven) and standardize fixtures on JSON.
- Restructure/expand tests across client and model modules; fix a couple of small model-mapping edge cases surfaced by tests.
File summaries
| File | Description |
|---|---|
tests/test_user.py |
Adds basic mapping tests for BirdBuddyUser. |
tests/test_sightings.py |
Characterizes sighting-report behavior (token decoding, finishing strategies, match selection). |
tests/test_sighting_create.py |
Removes standalone sighting-create tests (moved into test_client.py). |
tests/test_schema.py |
Adds drift-guard test ensuring referenced GraphQL types exist in the committed schema (with allowlist). |
tests/test_media.py |
Adds tests for media URL expiry parsing and media/collection model properties. |
tests/test_issue_40.py |
Removes issue-specific postcard-finishing tests (consolidated into test_client.py). |
tests/test_feeder.py |
Adds tests for feeder enums, metrics, location mapping, defaults, and update status. |
tests/test_feed.py |
Adds tests for feed node typing, datetime parsing, edges/nodes helpers, and filtering. |
tests/test_client.py |
Consolidates/parameterizes client tests (finish-postcard, sighting create/progress, reanalyze). |
tests/fixtures/schema.json |
Updates committed GraphQL schema fixture. |
tests/fixtures/postcard_sighting.json |
Adds JSON fixture replacing the prior YAML fixture. |
tests/fixtures/issue-40.yaml |
Removes YAML fixture (JSON standardization). |
tests/conftest.py |
Switches fixtures to JSON loader; renames fixture to postcard_sighting. |
scripts/dump_schema.py |
New script to introspect and write schema into tests/fixtures/schema.json. |
scripts/dump_payloads.py |
New script to capture raw + sanitized payload dumps for fixture creation. |
pyproject.toml |
Replaces pyyaml dev dep with python-dotenv for payload-dumper script. |
Makefile |
Updates schema target to run scripts/dump_schema.py. |
dump_schema.py |
Removes old root-level schema dumper script. |
birdbuddy/sightings.py |
Adds docstring to enum fallback; sighting behavior covered by new characterization tests. |
birdbuddy/media.py |
Fixes is_media_expired() to return None when Expires param is absent (instead of raising). |
birdbuddy/feeder.py |
Improves enum fallback docs; fixes owner feeder nested location mapping; fixes power_profile default behavior. |
birdbuddy/feed.py |
Adds docstring to enum fallback for unknown feed node types. |
.gitignore |
Ignores local raw/sanitized payload dump files. |
.env.example |
Adds example env vars for running scripts/dump_payloads.py. |
Review details
- Files reviewed: 24/26 changed files
- Comments generated: 2
- Review effort level: Low
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Good feedback all around, I'll work on getting those fixed 👍 |
scripts/dump_payloads.py logs in with BB_EMAIL/BB_PASSWORD (loaded from a local .env via python-dotenv) and captures the feeders, new postcards, and the postcard -> sighting flow. Each risky call is captured so a server error (e.g. the postcard INTERNAL_SERVER_ERROR) lands in the dump rather than aborting the run. It writes two git-ignored files: birdbuddy_payload.dump.json (the raw capture) and birdbuddy_payload.sanitized.json (the same data with a deterministic sanitizer applied -- UUIDs remapped with uuid5; email, names, serials, URLs and location replaced with stable fakes; opaque auth tokens blanked; the signed report token decoded, its embedded ids remapped, and re-encoded so it stays decodable; enums, numeric metrics, timestamps and public species names preserved), suitable for copying into tests/fixtures after review. .env.example documents the required BB_EMAIL/BB_PASSWORD.
Merge the issue/feature-named test_issue_40.py and test_sighting_create.py into a source-module-named test_client.py. Parameterize the best-guess / anomaly-correction finish cases and the in-progress / completed check-progress cases; hoist the repeated UUIDs into named constants. Rename the issue_40 fixture and issue-40.yaml to postcard_sighting. Behavior and coverage are unchanged.
Add tests/test_sightings.py pinning the current behavior of the complex sighting-report logic the postcard finishing flow relies on: SightingType classification, token_json (plain / signed / malformed), the finishing strategies (recognized, recognized-species propagation, mystery fallback), and highest_confidence_matches. Document SightingType._missing_, the fallback handler the unknown-type test exercises.
Cover the model classes that had no direct tests, built from dict fixtures with no network: - Species and BirdBuddyUser property mapping. - Feed / FeedNode / FeedNodeType: type resolution and the Unknown fallback, datetime parsing, and Feed.filter / newest_edge. - Media / Collection / is_media_expired: signed-URL expiry, image vs video, and collection species/visits/cover media. - Feeder enums (known values + UNKNOWN fallback), Signal, Battery, Feeder, and FeederUpdateStatus. Raises total coverage from 63% to 71%. Fix a non-breaking bug and clarify a related default the tests surface: - is_media_expired returns None for a signed URL with no Expires param, instead of raising KeyError. - Feeder.power_profile returns UNKNOWN explicitly for a missing profile; the old "STANDARD" default resolved to UNKNOWN anyway (it does not match the enum value "STANDARD_MODE"), so behavior is unchanged. Complete the Google docstrings for code the new tests exercise: a Returns: section on Feeder.power_profile and docstrings for the _missing_ enum fallback handlers in feeder.py and feed.py. Ruff's pydocstyle skips one-line and single-underscore-private methods, so the gate did not flag them.
Convert postcard_sighting.yaml to JSON and load fixtures via json; drop
the now-unused pyyaml dev dependency. The API, the payload dumper and
schema.json are all JSON, so the fixtures follow suit.
Sanitize postcard_sighting.json: remap every account identifier (feeder,
sighting and media ids) to generated UUIDs, and re-encode (rather than
blank) its JWT reportToken with scrubbed userId/feederId so the reportItems
the finish-flow tests read survive. Remap test_client.py's expected
constants to match. Drop the decoded_reportToken debug field: a plaintext
token decode added to the original fixture for readability, not part of the
API response.
Fix Feeder.location to read the owner-nested location{city,country} shape
as well as the flat locationCity/locationCountry keys; an owner feeder
previously yielded (None, None). Its docstring gains a Returns: section
for the (city, country) shape, and test_feeder.py covers both.
parrot-tailor
force-pushed
the
split/pr3-schema-tests
branch
from
July 22, 2026 23:46
2dbb098 to
381125f
Compare
Contributor
Author
|
Folded the requested changes into their respective commits, rebased, and updated the PR body. Let me know if I missed anything or if there's anything else you'd like to see changed here 🙇♂️ |
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
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.
Split of #40, part 3 of 3 (final). Per your review there, separating pure repo/tooling from formatting/linting and functional/test. On the surface, the lines added look high, but it's both an artifact of test files being reorganized to match their module names, and of a new helper script to dump live (and sanitized) payloads from the Bird Buddy API for manual testing and creating more realistic test fixtures going forward.
Once this lands, #40 can safely be nuked from orbit.
Commits
dump_schema.pytoscripts/andschema.jsontotests/fixtures/, drop the staleschema-20230225.txt, and addtests/test_schema.py, which derives the GraphQL types the library's queries use and asserts the committed schema defines them._KNOWN_MISSINGallowlist (see BB API update / increased number of internal errors from server #29). No breaking changes here; existing behavior preserved. Will fix the actual issue in a follow-up PR (started in Migrate the postcard flow to postcardCollect; remove the legacy sighting API #41, that needs to be reworked to have a series of smaller and non-breaking changes to allow for a deprecation cycle).scripts/dump_payloads.pylogs in with$BB_EMAIL/$BB_PASSWORD(loaded from a local.envfile viapython-dotenv), captures the feeders/postcards/sighting flow, and writes two git-ignored files: the raw dump plus a deterministically-sanitized copy.test_client.py— merge the issue/feature-named test files into one named based on the source module, parameterize the finish and check-progress cases, and rename the fixture topostcard_sighting; behavior and coverage unchanged.tests/test_sightings.pypinsSightingType,token_json(plain/signed/malformed), the finishing strategies, andhighest_confidence_matches.power_profiledefault (behavior unchanged: a missing profile resolves toUNKNOWN). Fixes a non-breaking bug the tests surface:is_media_expiredreturnsNonefor a URL with noExpiresparam instead of raisingKeyError.is_media_expired()change is non-breaking / safer usage for ha-birdbuddy: all call sites useis_media_expired()in a boolean context. It turns a would-be crash into a more gracefulNonethat the signature already promised. This is the proper fix for what Copilot flagged yesterday: Adopt the ruff/pyright/pytest toolchain and gate CI #44 (comment).pyyamldev dependency), sanitize the convertedpostcard_sighting.json, and remaptest_client.py's expected constants to match. Also fixesFeeder.locationto read the owner-nestedlocation{city,country}shape as well as the flat keys; an owner feeder previously yielded(None, None).Testing
make teston Python 3.10.20 passes: up to 49 tests, 71% coverage. The drift guard passes against a freshmake schemare-dump. No new account-derived fixtures yet;postcard_sighting.jsonis the sanitized JSON conversion of the pre-existingissue-40.yaml.