Add support for multiple rendering targets

lencioni commented 8 years ago

Currently Happo runs screenshots in one browser, that happens to be Firefox. While that is very useful, this project would be much more useful if it were able to be configured to run screenshots in multiple browsers and other rendering targets, such as React Native.

lencioni commented 8 years ago

It would also be great if it could be set up to run screenshots in multiple versions of the same browser.

lencioni commented 7 years ago

To do this properly, since some browsers are on different operating systems, we will need some sort of mechanism to aggregate multiple happo runs of results into a single end result that is to be uploaded. This aggregated view would probably be useful if it showed you diffs by example, with all of the render targets (e.g. browsers) represented so that you can both see how things have changed and also how they may differ between browsers. I don't know exactly what the UI should look like, but this seems like a good plan to me.

I think we should probably avoid running diffs on screenshots generated by different render targets--at least to begin with--because there will likely be discrepancies between things that don't really matter for this, such as anti-aliasing.

This aggregation mechanism should also be designed with parallelizing an individual rendering target's examples in mind, to help large projects scale better. For example, if I have 1000 examples and I want to make it go faster, there should be a way for me to tell one instance of happo to run the first half of the examples in firefox and a different instance of happo to run the second half of the examples in firefox, and then when those are done I should be able to aggregate the results. The command could be something like:

happo run firefox 1/2

or with more explicit flags (these could probably be named better):

happo run --target=firefox --split=1 --of=2

This would probably need to output some sort of useful JSON that the master process could use for its aggregation.

cc @lelandrichardson

trotzig commented 7 years ago

Here's an idea for how we could aggregate results:

Instead of serializing the result json into the uploaded html document, we provide a separate json file with the results. We then give each run the same id when we upload,

# on machine 1:
happo run --target firefox --split=1 --of=2
happo upload --id=xyz123

# on machine 2:
happo run --target firefox --split=2 --of=2 --id=xyz123
happo upload --id=xyz123

When upload receives an id, it will first download the previous json, merge in its own results, then upload again.

But wait, that won't work. There will be a race condition with other processes downloading and merging the same file. Hrm.

Okay, this should work better: we use the id as an indicator that we should only upload the result json, not the full html. The name of the json file could be resultSummary.${split}.json. We then need to mark one of the uploads as master (and perhaps the others as slaves?). The master will upload the html document with information about what json files to expect:

happo upload --master --id=xyz123 --splits=2

I guess we could also leave out the --splits=2 and assume up to 10 or so splits:

times(10).map((i) => window.fetch(`resultSummary.${i}.json`));

We would use the id in place of the random string in src/server/S3Uploader.js.

If we go with above solution, these should be the tasks involved:

[ ] Make <HappoDiffs> grab its images from a resultSummary.json instead of relying on json injected in the document.
[ ] Allow an id to override random folder name in S3Uploader.js.
[ ] Prevent upload of document on "slaves".
[ ] Support multiple resultSummary.${i}.json from <HappoDiffs>.

We'd have to somehow incorporate the target here too. Although maybe it's better not to mix different targets on a single review page? I guess that should be a decision to be made by the user though, so perhaps we should support mixing. In that case, there are a few more tasks to take care of as well.

Thoughts? 😃

lencioni commented 7 years ago

Why do the workers need to upload anything at all? They should be able to just report their resulting JSON back to the master process when they are done.

trotzig commented 7 years ago

True. I don't have too much experience setting up systems like these. I was mostly thinking that the upload from workers would be simpler than having to report back to a master. But I realize now that not reporting back to a master would make it trickier to know when things are done.

lelandrichardson commented 7 years ago

@lencioni and I were talking about this the other day. I wanted to memorialize some of the ideas we came up with...

I think there is a way we could make it so that every machine that was running was just responsible for their own RunResult (which can be from multiple targets but doesn't have to be).

The approach would be as follows. Let's say I have a PR that kicks off three separate CI processes. One on travis which runs the firefox target, another on circleci which runs the ios RN target, and another on circleCI which runs the android RN target. Each with some number of examples defined.

Before anything runs, we could deterministically know what files were going to be uploaded. The folder name would be uniquely determined by the SHA range:~/{startSHA}-{endSha}/ or something similar.

In that folder, we would have the following:

result-ios-target.json: a list of snapshots with diffs from ios. could be empty.
result-android-target.json: a list of snapshots with diffs from android. could be empty.
result-firefox-target.json: a list of snapshots with diffs from firefox. could be empty.
HappoApp.js: the JS for the viewer
review.html: essentially what is in review.ejs right now, with a minor tweak: it has some JS core in it that makes 3 ajax requests to get the 3 json files it needs, and then feeds that data into the HappoApp.js component, so that it knows which images to render and such.

This idea could be expanded to work with parallelized work all with the same target (ie, result-firefox-target-split-1.json) etc. The main idea is just that we have each machine running happo responsible for it uploading its own data, and then a static web app that is smart enough to just download whatever results are there and display them.

cc @trotzig

trotzig commented 7 years ago

This makes sense to me. I'm trying to think about ways this might be specific to s3 in any way, but I don't see any. Any happo "reporter" could be expected to deal with files somehow (upload them, store them in a file system, etc).

The approach would be as follows. Let's say I have a PR that kicks off three separate CI processes. One on travis which runs the firefox target, another on circleci which runs the ios RN target, and another on circleCI which runs the android RN target. Each with some number of examples defined.

Which process would be responsible for uploading the HappoApp.js and the html document? I guess that could be up to anyone really. If it's not been uploaded, upload it. If it's already there, ignore it and just upload the results.

Is reporter a good name for this? Then we would have happo-reporter-s3, happo-reporter-filesystem (see https://github.com/Galooshi/happo/issues/176 for context).

Galooshi / happo

Add support for multiple rendering targets #73