SDK bug fixes and test changes; v0.1.18 release#279
Merged
Conversation
fix test_load_config_file_found using tmp_path instead of CWD, convert module-level DRY_RUN/TEST_STATE_PERSISTENCE to fixtures, remove log.trace placeholders, replace blanket noqa with per-line, add assertion messages to bare asserts.
refactor FakeFileFactory generator-of-generator to plain list, cap temp_large_binary_file at fixed 10mb default, replace time.sleep(2) with polling loop, fix assert all() to provide detailed failure info, remove log.trace placeholders and unused helper functions, add assertion messages to bare asserts, unify responses fixture style, remove commented-out test stubs, replace blanket noqa with per-line suppressions.
split 1882-line test_uploads.py into test_uploads_workflow.py (32 workflow state machine tests) and test_uploads_persistence.py (38 persistence manager tests).
- add FBT001/FBT002 to per-file-ignores for test files (boolean positional args intentional in pytest fixtures) - fix E501 line-too-long: duplicated comment, split long f-strings - fix SLF001 noqa placement: cover both assert line and f-string continuation when both access private members
chmod on parent directory has no effect on open(path,'w') for an existing file - write permission on the file itself is needed. Changed parent.chmod(0o555) to file.chmod(0o444) in both test_remove_persisted_upload_handles_write_error and test_remove_persisted_uploads_by_checksum_handles_write_error.
regression from v0.1.15 anyio.Path migration: file discovery rglob yielded relative candidates from unresolved root, but resolved absolute root was passed to create_file_instance, causing Path.relative_to() to raise ValueError. fix by passing unresolved root (same source as rglob) instead.
- add pytest-xdist>=3.6.1 dev dependency - add heavy marker for large upload tests (>10 MB) - make test-integration run in parallel with --dist loadgroup - add test-integration-sequential for original single-process mode - add test-integration-quick to skip heavy tests - enable parallel coverage data file mode - bump version 0.1.17 -> 0.1.18
wrap the synchronous sds_files.upload_file call in asyncio.to_thread so the event loop can multiplex max_concurrent_uploads workers instead of being serialised by each blocking requests call
…t groups
- split integration_client into session-scoped object creation (no HTTP)
and function-scoped auth, so the fixture works with pytest-responses'
global pytest_runtest_setup hook that fires before session fixtures
- add xdist_group('rh_capture') to tests sharing RadioHound scan group
to prevent race conditions when tests delete each other's captures
- mark 100 MB upload parametrized case with heavy marker
lucaspar
added
ci/cd
lucaspar
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes SDK configuration/behavior regressions around HTTP request timeouts and upload workflows, and modernizes/parallelizes the SDK test suite (including xdist support) while updating docs and tooling for the v0.1.18 release.
Changes:
- Fix HTTP timeout propagation by mapping
HTTP_TIMEOUT→SDSConfig.timeout(defaulting to 300s) and using it inGatewayClient. - Fix upload workflow issues (relative-path discovery and async upload concurrency by offloading blocking uploads to a thread).
- Restructure and harden tests (split upload tests, add clearer assertions, add pytest-xdist + coverage parallel settings, improve integration test fixtures).
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/uv.lock | Adds xdist-related locked dependencies; bumps editable package version. |
| sdk/tests/test_uploads_workflow.py | New state-machine-focused tests for UploadWorkload. |
| sdk/tests/test_uploads_persistence.py | Refactors persistence tests; adds/adjusts assertions and error-path coverage. |
| sdk/tests/test_client.py | Adds regression tests ensuring HTTP timeout config is applied. |
| sdk/tests/ops/test_paginator.py | Cleans up placeholder logging; improves assertion messages. |
| sdk/tests/ops/test_files.py | Removes placeholder logging; adopts pytest-responses fixture injection; improves assertions. |
| sdk/tests/ops/test_captures.py | Converts module constants to fixtures for xdist safety; improves test parameterization. |
| sdk/tests/integration/test_file_ops.py | Updates passthrough endpoints, improves assertions, marks heavy test, replaces sleep with polling. |
| sdk/tests/integration/test_captures.py | Adds auth passthrough and xdist grouping; improves assertions; removes dead helper. |
| sdk/tests/integration/regressions/test_paths.py | Adds auth passthrough, improves failure reporting, adds xdist grouping. |
| sdk/tests/integration/integration.env.example | Updates local URLs for API-key generation. |
| sdk/tests/integration/conftest.py | Splits integration client creation vs authentication for pytest-responses/xdist compatibility; fail-fast on bad creds. |
| sdk/tests/conftest.py | Makes large-file fixture deterministic; simplifies fake file factory iteration. |
| sdk/src/spectrumx/gateway.py | Uses shared default HTTP timeout constant for GatewayClient. |
| sdk/src/spectrumx/config.py | Introduces DEFAULT_HTTP_TIMEOUT=300; fixes env key mapping so HTTP_TIMEOUT sets timeout. |
| sdk/src/spectrumx/api/uploads.py | Fixes relative-root discovery bug; offloads blocking upload calls via asyncio.to_thread. |
| sdk/pyproject.toml | Bumps version; adds pytest-xdist; adjusts coverage config for parallel runs; adds heavy marker and ruff test ignores. |
| sdk/justfile | Adds parallel integration test recipes and cleans .coverage.*. |
| sdk/docs/mkdocs/changelog.md | Updates v0.1.18 changelog content/date and adds fix notes. |
| sdk/.gitignore | Ignores agents/ directory. |
| sdk/.env.example | Updates HTTP_TIMEOUT example to 300 seconds. |
| network/justfile | Adds gen-certs recipe for self-signed TLS cert generation. |
| network/README.md | Documents the new gen-certs recipe usage. |
Comments suppressed due to low confidence (3)
sdk/tests/test_uploads_persistence.py:822
- This test uses
chmod(0o444)on the persisted uploads file to force a write error. That’s not reliable across platforms (notably Windows) and, unlessxdg_state_homeis patched, it writes into the real user/CI XDG state directory. Patchspectrumx.api.uploads.xdg_state_hometo atmp_pathlocation for isolation, and/or simulate the write failure by patching the specificPath.open(..., 'w')call to raiseOSErrorin a platform-independent way.
sdk/tests/test_uploads_persistence.py:992 - This test depends on
chmod(0o000)to trigger a read failure. File permission changes don’t behave consistently on Windows (and can be affected by the underlying filesystem/umask), which can make the assertion onlog_user_warningflaky. Prefer simulating theOSErrorby patchingPath.openfor this specific file, or skip/xfail this test on platforms where chmod-based permission errors aren’t reliable.
sdk/tests/test_uploads_persistence.py:1018 - This test uses
chmod(0o444)to force a write error, which is not consistently enforced on Windows and can lead to flaky behavior (the file may still be writable via ACLs). Prefer a deterministic approach (e.g., patch thePath.open(..., 'w')call for this file to raiseOSError) or guard the chmod-based approach with a platform check.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
klpoland
approved these changes
Apr 30, 2026
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.
SDK bug fixes:
gateway.py):Client.timeoutkwarg was ignored; requests always used default timeoutuploads.py): relative paths resolved incorrectly in persistence manageruploads.py): sync requests (checksum) blocked the async event loop — offloaded toanyio.to_thread.run_syncPath.openmonkey-patches withchmodpermission controlFaster integration tets:
pytest-xdistdependency; split auth fixture into session-scoped client creation + function-scoped auth to avoidresponsesinterception; added xdist groups for RadioHound tests sharing sample data.envmissing or invalidTest quality:
test_uploads.py(1400+ lines) split intotest_uploads_workflow.py(state machine tests) andtest_uploads_persistence.py(persistence tests)assertstatements across ops testsMagicMock(return_value=...)attribute assignment →.return_value = ...pattern@responses.activatedecorator →responsesfixture injection (pytest-responses pattern)loguruplaceholder tracesOther:
network/justfile)