test: Add multi-copy upload testing scenario with filecion-pin#103
Merged
Conversation
galargh
changed the title
galargh
changed the base branch from
galargh/last-known-good-ref
to
galargh/latest-synapse-sdk-localnet
galargh
commented
May 4, 2026
| return value or "" | ||
|
|
||
|
|
||
| def patch_synapse_core_streaming_upload(npm_dir: Path) -> bool: |
Contributor
Author
There was a problem hiding this comment.
This patch is needed because @filoz/synapse-core 0.4.1 sets Content-Length on a transformed streaming upload body. Node/Undici rejects that as invalid content-length header, and filecoin-pin surfaces it as StorageContext store failed: Failed to store piece on service provider - Network request failed.
galargh
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new end-to-end scenario test that validates multi-copy uploads against the devnet using filecoin-pin, then retrieves from multiple URLs and verifies the downloaded bytes against the expected root CID.
Changes:
- Introduces
scenarios/test_multi_copy_upload.pyto run a deterministic 20 MiB upload viafilecoin-pin, parse output (root CID / piece CID / retrieval URLs), download each retrieval URL, and verify content. - Updates
scenarios/run.pyto include the new scenario and adjusts thetest_storage_e2etimeout. - Updates PR CI workflow trigger behavior by removing the
pull_request.branches: ['main']filter.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| scenarios/test_multi_copy_upload.py | New multi-copy upload + retrieval verification scenario using filecoin-pin with a temporary npm project and runtime patching. |
| scenarios/run.py | Registers the new scenario in the ordered runner and updates timeouts. |
| .github/workflows/ci_pull_request.yml | Allows PR CI to run without restricting pull_request triggers to main. |
- make synapse-core Content-Length patch idempotent and document the upstream bug it papers over; only fail when the target file is missing - assert npm (test only uses npm), not pnpm - download verified payloads into a tempdir rather than ./ipfs/ - drop dead 'if not run_cmd' early-return branches (helpers.run_cmd already calls sys.exit(1) on failure) - assert URL substitution actually changed each retrieval URL before rewriting /piece/<piece_cid> to /ipfs/<root_cid>
rvagg
approved these changes
May 7, 2026
Contributor
rvagg
left a comment
rvagg
left a comment
There was a problem hiding this comment.
I added some changes here, Copilots and some extra ones
github-project-automation
Bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FOC
rvagg
added a commit
that referenced
this pull request
May 7, 2026
Co-authored-by: Rod Vagg <rod@vagg.org>
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.
Resolves #94
Description
Adds
test_multi_copy_uploadto the scenario suite. The test exercisesfilecoin-pinagainst the devnet by uploading a deterministic 20 MiB random file, requiring multiple retrieval URLs, downloading each retrieval URL, and verifying each downloaded file against the expected root CID.This PR is based on
galargh/latest-synapse-sdk-localnetfrom PR #107, which moves the devnet to the newer Synapse-compatible localnet setup.Current Test Flow
filecoin-pin=0.20.1multiformats=13.4.2@filoz/synapse-corepackage to removeContent-Lengthfrom the streaming upload request headers indist/src/sp/upload-streaming.js.filecoin-pin=0.20.1resolves to@filoz/synapse-core=0.4.1, and that published package still sendsContent-Lengthon the transformed streaming body.StorageContext store failed: Failed to store piece on service provider - Network request failed.filecoin-pin add --network devnet <upload_dir>filecoin-pinoutput before parsing.?format=raw.Other Changes
scenarios/run.py.test_storage_e2etimeout to200s.pull_request.branches: ['main']workflow filter so PR CI can run against the non-main PR feat: support latest synapse #107 base.Validation
Local:
CI:
8a7558b: https://github.com/FilOzone/foc-devnet/actions/runs/25344079405