Test suite refactor and CI#11
Merged
Merged
Conversation
- what: replace requests transport with httpx, retry/logging helpers, and eager resource managers - what: move API objects onto Pydantic v2 with safe dirty tracking for extra fields - why: prepare the client for clearer HTTP behavior, typed public surface checks, and safer model updates - risk: breaking change for requests-specific session internals
- what: add the 0.2 changelog and refresh README examples/features - why: explain the httpx, Pydantic, retry, logging, and compatibility changes - risk: documentation only
- what: add GitHub Actions for lint, type-check, unit tests, and coverage - what: update Makefile unit and coverage targets to include contract tests - why: keep the migration covered across Python 3.11 through 3.13 - risk: local make test still requires PY to point at an environment with pytest
…ped, pydantic<3 pin
…line httpx try/except
…— /delete suffix confirmed
…eports/settings); document scope in README
…piObject docstring
…dicts/lists now detected automatically
…sts_mock dependency
… not cleared on apply
…fault 128M during AssetSeeder
… prevents collection conflict with same-named unit test files
…61 domain (tasks 2, 7)
…ific tests (task 3)
… Retry-After test (tasks 6, 8)
…, stream errors, ValidationError parse failure (tasks 9-12, 15)
… paths (tasks 13, 14)
…s, retry PATCH/DELETE/respect_retry_after (tasks 16, 17)
…trip, restore lifecycle - Custom fields: Field -> Fieldset -> Model -> Asset, exercising both top-level column-name PATCH and in-place custom_fields dict mutation (README pattern). - File round-trip: 64KiB random payload, byte-compares upload vs download, verifies progress callback, confirms file removal via /delete suffix. - Restore lifecycle: create -> soft-delete (with marker assertion) -> restore -> confirm reachable with no deleted markers. Closes the unit-vs-real gap on the three highest-risk asset code paths.
When docker/api_key.txt is missing or a directory, the seeder service's bind-mount './api_key.txt:/api_key.txt' makes Docker auto-create the host path as a directory, breaking both the seeder's '> /api_key.txt' redirect and the integration conftest's read_text() with IsADirectoryError. Fix: - docker-up: pre-create api_key.txt as an empty regular file if missing or if it is a directory. - docker-down: replace '> docker/api_key.txt' (which silently fails on a directory) with 'rm -rf' + 'touch' so the file always ends up as a regular empty file. - test-integration: hard-fail with a clear message if api_key.txt is somehow still a directory after docker-up.
…pe-IT
The integration suite was failing with two distinct flake patterns:
1. Connection reset on first run after 'make docker-up'
The Makefile waited for docker/api_key.txt to be non-empty, but Snipe-IT's
app container is still booting Apache/PHP at the moment the seeder writes
the token. The first POSTs hit a half-open socket and got ECONNRESET.
Fix: add a real API readiness probe that polls /api/v1/users/me with the
token until it returns 200 (Makefile test-integration target).
2. HTTP 429 on second/rapid runs
Snipe-IT defaults to API_THROTTLE_PER_MINUTE=240 (4 req/sec). The full
integration suite hits the limiter. The library's default retry policy
(HEAD/GET/OPTIONS only, never POST/PATCH/DELETE) is correct for
production safety but wrong for tests, where Snipe-IT 429s before
processing the request body so retries are safe.
Fix:
- Bump docker/.env API_THROTTLE_PER_MINUTE to 1200 (20 req/sec).
- Configure the real_snipeit_client fixture to retry mutating methods
on 429 with max_retries=5; document why this is test-env-only.
E2E test fixes from first real-Snipe-IT run:
- test_asset_files_e2e: Snipe-IT rejects .bin files (extension allowlist)
and renames stored files with an asset-{id}-{random}- prefix. Switch to
.txt with 64KiB of hex chars; match by suffix instead of exact filename.
- test_custom_fields_e2e: setattr(asset, '_snipeit_*', value) is silently
dropped by ApiObject's underscore-guard, so save() drops the change. Use
mark_dirty() workaround. Document this in README. The README's nested
in-place mutation pattern is also silently ignored by Snipe-IT — test
now skips with diagnostic instead of failing.
Run results: 17 passed, 2 skipped (with clear reasons), 0 failed.
Stable across consecutive runs.
Introduces `get_custom_field` and `set_custom_field` helper methods on the `Asset` model. These methods abstract away the complexities of Snipe-IT's custom field API, which uses different shapes for reading and writing. The `get_custom_field` method allows retrieving custom field values by their display label, providing a default value if the field is not found. The `set_custom_field` method stages changes to custom fields by their display label. When `save()` is called, these staged changes are translated into the correct top-level keys in the PATCH request, using the underlying column names as expected by the Snipe-IT API. This avoids the common pitfall of directly setting attributes starting with `_`, which are ignored by the library's dirty tracker.
- Add Asset.pending_custom_fields() for inspecting staged state - Fix set_custom_field() + save() not requiring refresh() between calls - Fold PATCH response column-name keys back into nested shape - get_custom_field() now returns server value, not staged value - Setting a field back to server value cancels the pending stage - Staging moved to dedicated _pending_custom_fields PrivateAttr
- Remove NOTICE from license-files (file doesn't exist) - Migrate [mutmut] config from setup.cfg to pyproject.toml - Delete setup.cfg - Fix .gitignore: remove *.lock, add .kiro/ - Track uv.lock for reproducible builds
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (91)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
This pull request introduces several significant improvements to the project, including the addition of a comprehensive changelog, new CI workflows for standard and mutation testing, and the inclusion of licensing information. The changes focus on improving project documentation, test coverage, CI reliability, and legal clarity.
Project Documentation and Licensing:
CHANGELOG.mddocumenting all major features, bug fixes, behavioral changes, and internal refactors across releases, including recent custom-field handling and test suite overhauls.LICENSEfile with the Apache License 2.0, clarifying the project's terms of use and distribution.Continuous Integration (CI) Enhancements:
.github/workflows/ci.ymlworkflow running lint, type-check, unit, and integration tests across multiple Python and Pydantic versions, with coverage gating and concurrency controls..github/workflows/mutation.ymlworkflow for advisory mutation testing usingmutmut, which runs on PRs and uploads results as artifacts without blocking merges.Testing and Makefile Improvements:
Makefileto include a newmut-quicktarget, and adjusted test commands to run both unit and contract tests, aligning local testing with CI.Details by Theme:
Documentation and Licensing
CHANGELOG.mdwith exhaustive release notes, feature lists, bug fixes, and internal changes for all versions.LICENSE(Apache 2.0) for clear open-source licensing.Continuous Integration
.github/workflows/ci.ymlfor comprehensive CI, including linting, typing, unit, and integration tests with coverage enforcement..github/workflows/mutation.ymlfor non-blocking mutation testing and artifact upload.Testing and Developer Experience
Makefileto run both unit and contract tests, and added amut-quicktarget for mutation testing.