{"action":"created","issue":{"url":"https://api.github.com/repos/mattheworiordan/capybara-screenshot/issues/121","labels_url":"https://api.github.com/repos/mattheworiordan/capybara-screenshot/issues/121/labels{/name}","comments_url":"https://api.github.com/repos/mattheworiordan/capybara-screenshot/issues/121/comments","events_url":"https://api.github.com/repos/mattheworiordan/capybara-screenshot/issues/121/events","html_url":"https://github.com/mattheworiordan/capybara-screenshot/pull/121","id":60419400,"number":121,"title":"WIP: S3 upload","user":{"login":"mwise","id":1072,"avatar_url":"https://avatars.githubusercontent.com/u/1072?v=3","gravatar_id":"","url":"https://api.github.com/users/mwise","html_url":"https://github.com/mwise","followers_url":"https://api.github.com/users/mwise/followers","following_url":"https://api.github.com/users/mwise/following{/other_user}","gists_url":"https://api.github.com/users/mwise/gists{/gist_id}","starred_url":"https://api.github.com/users/mwise/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mwise/subscriptions","organizations_url":"https://api.github.com/users/mwise/orgs","repos_url":"https://api.github.com/users/mwise/repos","events_url":"https://api.github.com/users/mwise/events{/privacy}","received_events_url":"https://api.github.com/users/mwise/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":3,"created_at":"2015-03-09T22:12:51Z","updated_at":"2015-03-10T00:18:05Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mattheworiordan/capybara-screenshot/pulls/121","html_url":"https://github.com/mattheworiordan/capybara-screenshot/pull/121","diff_url":"https://github.com/mattheworiordan/capybara-screenshot/pull/121.diff","patch_url":"https://github.com/mattheworiordan/capybara-screenshot/pull/121.patch"},"body":"Ok, so this isn't ready to be merged yet, but I wanted to get eyes on it before I got too far along. I made the S3 integration opt-in as much as possible, but I couldn't find a clean way to avoid adding the `aws-sdk` to the Gemfile short of creating a separate gem.\r\n\r\nA few things to note:\r\n* For frameworks that support an 'after suite' hook (RSpec, MiniTest, Spinach), I am batching the uploads and processing them at the end of the run. For the others, I'm uploading them inline since I didn't see a clear way to do them on exit. I need to clean up the `enqueue/flush` strategy for these cases and can probably streamline it to just upload the files inline.\r\n* Calling `screenshot_and_save_page` and `screenshot_and_open_image` doesn't upload the file to S3 by default, since I thought this would be the most common use case. It would seem that the S3 uploads are really only valuable in a CI context. Suggestions welcome on this.\r\n* I didn't add the 'remove files from the filesystem' flag yet, but I think this could be easily done.\r\n\r\nOne question:\r\n* Where are the `check_file_content` and `check_file_presence` spec helpers defined? I couldn't find them in the source, and adding the above 'remove files from the filesystem' option would break the specs that test for those files.\r\n\r\nSo yeah, please take a look at the changes and let me know what you think. It's working as-is with our RSpec-based suite, but likely quite a bit more testing would be required for the other frameworks. \r\n\r\nCheers,\r\n\r\nMark"},"comment":{"url":"https://api.github.com/repos/mattheworiordan/capybara-screenshot/issues/comments/77972540","html_url":"https://github.com/mattheworiordan/capybara-screenshot/pull/121#issuecomment-77972540","issue_url":"https://api.github.com/repos/mattheworiordan/capybara-screenshot/issues/121","id":77972540,"user":{"login":"mwise","id":1072,"avatar_url":"https://avatars.githubusercontent.com/u/1072?v=3","gravatar_id":"","url":"https://api.github.com/users/mwise","html_url":"https://github.com/mwise","followers_url":"https://api.github.com/users/mwise/followers","following_url":"https://api.github.com/users/mwise/following{/other_user}","gists_url":"https://api.github.com/users/mwise/gists{/gist_id}","starred_url":"https://api.github.com/users/mwise/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mwise/subscriptions","organizations_url":"https://api.github.com/users/mwise/orgs","repos_url":"https://api.github.com/users/mwise/repos","events_url":"https://api.github.com/users/mwise/events{/privacy}","received_events_url":"https://api.github.com/users/mwise/received_events","type":"User","site_admin":false},"created_at":"2015-03-10T00:18:05Z","updated_at":"2015-03-10T00:18:05Z","body":"@mattheworiordan after spending a little time with it today, I do think it should be a separate gem. The aws-sdk dependency is definitely a weird one to include for all users. I think it should be easy enough to roll as a psuedo-plugin - hopefully without the need for an update to capybara-screenshot. \r\n\r\nThanks for the quick feedback and the tip about `check_file_content` - apparently that gem needs to work on it's SEO!"}}