feat(netlify): upgrade to SDK 2.0.0 with unit + integration tests#386
feat(netlify): upgrade to SDK 2.0.0 with unit + integration tests#386Tram-Nguyen87 wants to merge 2 commits into
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
🔍 Integration Validation ResultsCommit: Changed directories:
✅ Structure Check output✅ Code Check output✅ Tests Check output✅ README Check output✅ Version Check output |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5864610731
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| set in the NETLIFY_ACCESS_TOKEN environment variable. | ||
|
|
||
| Run all read-only tests: | ||
| pytest netlify/tests/test_netlify_integration.py -m integration |
There was a problem hiding this comment.
I checked python -m pytest --help; -m runs tests matching the mark expression, so -m integration also selects the classes below that are both integration via the module pytestmark and destructive. Anyone following the advertised “read-only” command with NETLIFY_ACCESS_TOKEN set will create/update/delete real Netlify sites; use -m "integration and not destructive" for the read-only command.
Useful? React with 👍 / 👎.
| upload_url = upload_call.args[0] if upload_call.args else upload_call.kwargs.get("url", "") | ||
| assert upload_call.kwargs.get("method") == "PUT" | ||
| assert "deploy-2" in upload_url | ||
| assert sha1 in upload_url |
There was a problem hiding this comment.
When Netlify returns required, those entries are SHA1 digests used to decide which file contents are needed, but the upload request must target /deploys/{deploy_id}/files/{file_path} (for example, /files/index.html) per Netlify's API guide. This new assertion makes the unit suite pass the digest-in-path URL that the real API rejects whenever a file upload is required, so please assert the original file path instead.
Useful? React with 👍 / 👎.
… tests from read-only command
- Upload URL now correctly uses the file path (/deploys/{id}/files{/path})
instead of the SHA1 hash per the Netlify API spec; added hash_to_path
mapping to track which path corresponds to each required SHA1
- Read-only integration test command corrected to -m "integration and not
destructive" so destructive lifecycle tests are not accidentally run
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TheRealAgentK
left a comment
TheRealAgentK
left a comment
There was a problem hiding this comment.
Posted one blocker from the integration-test review checklist.
|
|
||
| pytestmark = pytest.mark.integration | ||
|
|
||
| ACCESS_TOKEN = os.environ.get("NETLIFY_ACCESS_TOKEN", "") |
There was a problem hiding this comment.
🚫 Blocker: this new live integration test reads NETLIFY_ACCESS_TOKEN, but the PR doesn't add NETLIFY_ACCESS_TOKEN= to the root .env.example. Per the integration-test checklist, every env var read by integration tests must be documented there so reviewers and future contributors have a canonical setup contract. Please add a Netlify block with this variable before merge.
TheRealAgentK
left a comment
TheRealAgentK
left a comment
There was a problem hiding this comment.
Requesting changes because the new live integration tests read NETLIFY_ACCESS_TOKEN but the root .env.example setup contract is missing that variable.
2 participants
Summary
autohive-integrations-sdk~=1.0.2to~=2.0.0context.fetch()return values updated to access.datafor the bodyActionResultwith error data toActionError(message=...)result: boolanderrorfields removed from all output schemas inconfig.json;delete_siteoutput updated todeleted: booleanconfig.jsonversion bumped to2.0.0deploy_idNone guard increate_deployto fail fast with a clear error rather than silently making requests to/deploys/None/...tests/test_netlify.py(manual async runner) andtests/context.pywith a proper pytest suiteTests
Unit tests (
test_netlify_unit.py) — 42 testslist_sites,create_site,get_site,update_site,delete_site,list_deploys,create_deploy,get_deployActionErroron exception,VALIDATION_ERRORon missing required inputscreate_deployedge cases: no required files, file upload with correct deploy_id + SHA1 in URL +Content-Type: application/octet-streamheader,deploy_urlpriority (deploy_ssl_url->ssl_url->url), multiple file hashing, missing deploy ID in API responseIntegration tests (
test_netlify_integration.py) — 10 tests (run against live Netlify API)TestListSites,TestGetSite,TestListDeploys,TestGetDeploy-m "integration and destructive"real_fetchraisesHTTPErroron non-2xx to mirror SDK production behaviourTest plan
python -m pytest netlify/tests/test_netlify_unit.py -v— 42 passedpython -m pytest netlify/tests/test_netlify_integration.py -m integration -v— 10 passed (live API, including destructive lifecycle)validate_integration.py netlify— 0 errors, 0 warningscheck_code.py netlify— lint, format, security, imports, fetch patterns, config-code sync all green/code-reviewhigh effort self-review run pre-PR — all findings fixed before submissionGenerated with Claude Code