Galooshi / happo

Visual diffing in CI for user interfaces
504 stars 16 forks source link

Generate review html page #176

Closed tleunen closed 7 years ago

tleunen commented 7 years ago

I'm trying to add Happo on a project integrated with CircleCi, and instead of using the upload to S3 feature, I'd like to use the artifacts for each circleci build.

Which means that I'd like the happo review to generate the webpage in a specific directory so that I can just push this directory in the artifact, instead of having it running a webserver.

Would it be possible to generate the html webpage alongside resultSummary.json? If not, maybe have a new option to happo review?

trotzig commented 7 years ago

Okay, just to make sure I understand this (I'm not too familiar with CircleCI). When a happo run is done, you want to generate the "review" page and store it in a local folder?

(Just read up on CircleCI artifacts, and the $CIRCLE_ARTIFACTS env variable, so I think we're on the same page).

My first idea would be to (ab)use happo upload instead of happo review, and implement a new "Uploader". The S3 part of uploading is well isolated, so we'd just have to implement something that uses the same interface as S3Uploader.js (https://github.com/Galooshi/happo/blob/68141002a621ccd859715fad2ad67e67266c4e53/src/server/S3Uploader.js).

We'd have to configure this somehow as well, and I wonder if we should add a uploader configuration option, where you can initialize either an S3Uploader, a FileSystemUploader, or something completely different.

// .happo.js
const S3Uploader = require('happo/server/S3Uploader');

module.exports  = {
  uploader: new S3Uploader({ 
    accessKey: ...,
    ...
  }),
}
// .happo.js
const FileSystemUploader = require('happo/server/FileSystemUploader');

module.exports  = {
  uploader: new FileSystemUploader({ 
    folder: process.env.CIRCLE_ARTIFACTS,
  }),
}

I'm about to push another happo version (hopefully today) so I can do some prep work, and perhaps leave the implementation of FileSystemUploader (there's probably a better name) to you. Sounds good?

trotzig commented 7 years ago

I pushed v4.0.0-beta.1 which will allow you to implement a custom uploader through configuration. Mimic the interface implemented by S3Uploader and it should work ™️.

tleunen commented 7 years ago

you want to generate the "review" page and store it in a local folder?

Exact. This way, it can be stored and reviewed without running the happo server.

It seems the upload command only upload the diff images, without any html page to easily see/compare them. Is that right? Would this be also customizable by the uploader?

leave the implementation of FileSystemUploader (there's probably a better name) to you. Sounds good?

Yep. I'll take a look

lencioni commented 7 years ago

I think we might want to consider a plugin architecture for upload target integration, like we have been discussing for CI and code review integration.

On Wed, Jan 4, 2017, 6:41 AM Tommy notifications@github.com wrote:

you want to generate the "review" page and store it in a local folder?

Exact. This way, it can be stored and reviewed without running the happo server.

It seems the upload command only upload the diff images, without any html page to easily see/compare them. Is that right? Would this be also customizable by the uploader?

leave the implementation of FileSystemUploader (there's probably a better name) to you. Sounds good?

Yep. I'll take a look

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Galooshi/happo/issues/176#issuecomment-270386041, or mute the thread https://github.com/notifications/unsubscribe-auth/AAL7zixAZM1I9pPW6KKswaSHJrTuqgpCks5rO6-VgaJpZM4LaRzf .

trotzig commented 7 years ago

It seems the upload command only upload the diff images, without any html page to easily see/compare them. Is that right? Would this be also customizable by the uploader?

There's an html file being generated as well.

I think we might want to consider a plugin architecture for upload target integration, like we have been discussing for CI and code review integration.

Yeah, probably good to note that it is likely that this will not be the final solution. It might be a good step on the way there though.

trotzig commented 7 years ago

@tleunen did you happen to use this configuration option?

tleunen commented 7 years ago

No, not yet :( I'm missing some time right now but I hope I can get back to it soon.

trotzig commented 7 years ago

No worries, thanks for responding though. :)