Comment 1:

Status changed to Started.

Comment 2:

it's high time we started with that, so we can test the importers output in the UI with
less pain...
https://camlistore-review.googlesource.com/3151

Comment 3:

I don't think we should modify the importer code at all.

Comment 4:

hmm, then I misunderstood what "optional interface on Importer" meant. Can you explain
further please?

Comment 5:

for the optional interface, you proposed:
type TestDataMaker interface {
    MakeTestData(bs blobserver.Storage) error
}
I don't think I understand where you want to go with the signature of MakeTestData, so
I'd rather discuss it before going further.
If MakeTestData eventually calls Run(rc *importer.RunContext), it will need to create
(or be given) a RunContext, because that's what the signature of Run wants, and that's
what the run uses to get everything it needs from the importer pkg. Like when it calls
r.Host.BlobSource(), or r.RootNode().
So, among other things, we need to have a *Host for our RunContext.
However, iiuc, we're not supposed to create a *Host for each importer impl, there's one
for all of them. So we should rather pass a *Host to MakeTestData, which will use it to
create a RunContext, no? Like so:
func (im *imp) MakeTestData(h *import.Host) error {
    rc := importer.NewRunContext(h) // to be created in importer.go
    // setup responses
    // ...
    
    tr := test.NewFakeTransport(responses)
    rc.Context = context.New(context.WithHTTPClient(&http.Client{Transport: tr}))
    return im.Run(rc)
}
or we could even directly pass a RunContext to MakeTestData, like so:
func (im *imp) MakeTestData(rc *importer.RunContext) error
If yes, comes the question of where we get the *Host (that devcam will pass to
MakeTestData). Do we create a new one, from a bunch of parameters (like a bs
blobserver.Storage), or do we reuse the one already created by camlistored at the
importer's init (newFromConfig) ?

Comment 6:

Perhaps the optional interface on an importer should simply be:
type TestDataMaker interface {
   MakeTestData() http.RoundTripper
}
And it'll return the fake HTTP transport.
Then the thing driving the test can make the importer.Host + contexts and call the
importer's normal Run method.

Comment 7:

yeah, I think that could work. on it.

Comment 8:

ok, that plan looks like it's working. rough draft in patchset 3 of
https://camlistore-review.googlesource.com/3151

Comment 9:

Since the *Host is at the core of the importer pkg, it looks like making a "remote"
*Host requires making some substantial changes, like using a *client.Client for its
target (blobserver.StatReceiver), as well as for its blobSource (blob.Fetcher), and
signer (*schema.Signer). Worse, it has a search Handler, which I think is used during
the Run, so this one would have to be swapped for a client that does search queries too.
This is starting to look like the publisher as a server app migration work.
I've started but it does not look like going the right way so far...
At this point, I think it'd be less disruptive to simply add an
(undocumented/hidden/debug) http access point (like the "populate" I had done in
https://camlistore-review.googlesource.com/#/c/3151/2/pkg/importer/importer.go) that
would trigger the makeThings(). And devcam server would just do an http request on it
once camlistored is up.
Or maybe a debug --makethings flag to camlistored ?
Or maybe there's a way to break Host into more reusable pieces...
Just letting you know early since you don't want this to become a big thing.

Comment 10:

Alright, I have something, albeit gross for now, working from devcam, and I didn't have
to take care of setting/replacing the *Host search handler for the Run to complete, so
maybe it's good enough.
Will post CL after some cleaning up.

Comment 11:

Well, a *client.Client does implement all those interfaces I think?

Comment 12:

https://camlistore-review.googlesource.com/3236

Comment 13:

Done for all four importers:
409ec362e3c0db0e8228afab12535e554480e44d
2e939b0b35171e6498d8b3452f74583566f87d96
f81b88de6e3deda26161e316a89007c006ef4e54
b82459424d45450bf0363188d7d038a5a98898ff
aa3cda0762ae94a75383bfbfb131beabbbf9ef66

Status changed to Fixed.