Skip to content

refactor: doupload uses Smelt#404

Open
Peeja wants to merge 1 commit into
feat/user-agentfrom
refactor/smelt
Open

refactor: doupload uses Smelt#404
Peeja wants to merge 1 commit into
feat/user-agentfrom
refactor/smelt

Conversation

@Peeja

@Peeja Peeja commented Apr 16, 2026

Copy link
Copy Markdown
Member

Closes #387

PR Dependency Tree

This tree was auto-generated by Charcoal

@Peeja Peeja changed the base branch from main to feat/user-agent April 16, 2026 20:39
@Peeja Peeja marked this pull request as ready for review April 16, 2026 20:43
@frrist

frrist commented Apr 16, 2026

Copy link
Copy Markdown
Member

Love that we're integrating with Smelt here. Before this lands, I want to change the shape of the test itself:

I'd like doupload rewritten as a Go integration test — a _test.go file that imports Smelt's pkg/stack to spin up the network and drives the guppy container via Smelt's guppy.Client — rather than a bash script that shells out to docker compose.

This is the pattern I want us to adopt for integration tests across all our services; Piri has already landed its first one (smoke_test.go), and guppy is the natural next place to apply it. This is a failure on my part. I had this direction in my head and should have written it down and shared it before you started on this PR - I'm sorry for the churn, but I think it'll be worth ❤️.

Why

  1. Team fluency. We're a Go shop/rapidly becoming one. I'm far better at Go than bash, and I suspect that's true across the team. The current doupload is doing enough heavy lifting (mprocs orchestration, jq heredoc config generation, regex/awk CID extraction, /dev/fd/3 tees) that touching it confidently is a high bar imo.
  2. Forcing function for structured CLI output. A Go client is how we get the CLI to emit JSON. Today both bash and Smelt's guppy client regex-match plain ASCII, and either breaks quietly on a flag rename, as an example. But bash tests allow us to paper over brittle output formats; a typed Go client gives us somewhere the pain lands, and a concrete consumer to migrate to --output json behind (this componds as we'll want a similar migration for sprue, piri, indexing-service, etc.).
  3. Assertions and tooling we already know. stretchr/testify, t.Cleanup, t.TempDir, table tests, -run filters, etc. We'll reinvent most of this in bash, and we'll get some of it wrong. Bugs in tests are bad on their own; doing it all inside a container already adds fragility, and I don't want to stack more on top.
  4. Smelt is designed for this. pkg/stack exists specifically so that nobody testing against the Forge stack has to leave Go.

How I'd like this to look 🙏

Port doupload to a Go test. The scenarios to preserve from the bash version: login, space generation, upload, upload check, verify, retrieve (full tree), retrieve (subdirectory), and diff against the source data.

A few capabilities the bash exercises aren't on guppy.Client yet, so this will include some Smelt SDK work:

  • guppy upload check — not on the interface
  • guppy verify — not on the interface
  • Richer GenerateTestData (two directories, min/max file sizes — today it takes a single size)
  • Retrieve with a subdirectory suffix (e.g. rootCID/small-files)
  • An equivalent to the email-clicker sidecar — likely a WaitForLoginURL helper on the SDK so tests don't need to tail container logs by hand

Happy to pair on any of the SDK extensions and to review the Smelt PRs quickly so this doesn't stall. Flag anything that turns out to be a bigger rock than expected.

@frrist frrist left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

requesting changes for reasons in my comment.

@alanshaw alanshaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really cool, but I think I agree with @frrist - it's likely going to be better for this test to be written in Go. It'll also reduce and centralize (in a good way) a lot of the configuration that you've had to add here, and by using the smelt library we'll reduce repeated code in other (Go) projects (upload service, egress tracker, delegator etc.) and reduce maintenance tasks when that common code changes.

@Peeja

Peeja commented Apr 17, 2026

Copy link
Copy Markdown
Member Author

Ahhh, I love this! That's way better than all this junk!

I'm feeling too deep in my stack, so I'm going to use this to test the actual Guppy changes I was working on, I just won't merge this yet. Then I'll move it to Go.

Peeja added a commit that referenced this pull request Apr 17, 2026
#### PR Dependency Tree


* **PR #405** 👈
  * **PR #404**

This tree was auto-generated by
[Charcoal](https://github.com/danerwilliams/charcoal)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants