Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove dependency on remote images in tests #859

Merged
merged 1 commit into from
Jul 14, 2017
Merged

Remove dependency on remote images in tests #859

merged 1 commit into from
Jul 14, 2017

Conversation

pustomytnyk
Copy link
Contributor

Fixes issue with images accessibility in tests (from #819). Issue can be reproduced with limiting internet bandwidth (test still pass if I've fully disabled internet).

@@ -32,3 +32,9 @@ module Features

ActiveRecord::Migration.maintain_test_schema!
Capybara.javascript_driver = :poltergeist
Capybara.register_driver :poltergeist do |app|
options = {
phantomjs_options: ['--load-images=no']

Choose a reason for hiding this comment

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

Put a comma after the last item of a multiline hash.
Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@nickcharlton nickcharlton modified the milestone: v0.8.0 May 8, 2017
@krzysiek1507
Copy link
Contributor

Hi @pustomytnyk could you please rebase it? Thanks!

@nickcharlton
Copy link
Member

Hi @pustomytnyk! If I could get you to fix the two Hound comments, I'd be happy to merge this.

options = {
phantomjs_options: ['--load-images=no']
}
Capybara::Poltergeist::Driver.new(app, options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not just pass the options directly here?

@pustomytnyk pustomytnyk reopened this Jun 24, 2017
@@ -32,3 +32,6 @@ module Features

ActiveRecord::Migration.maintain_test_schema!
Capybara.javascript_driver = :poltergeist
Capybara.register_driver :poltergeist do |app|
Capybara::Poltergeist::Driver.new(app, phantomjs_options: ["--load-images=no"])

Choose a reason for hiding this comment

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

Line is too long. [81/80]

@pustomytnyk
Copy link
Contributor Author

pustomytnyk commented Jun 24, 2017

updated.

Why not just pass the options directly here?

Hound complained about line being too long (81/80).

@nickcharlton
Copy link
Member

Got it, thanks! Merging.

@nickcharlton nickcharlton merged commit 4e7b440 into thoughtbot:master Jul 14, 2017
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.

5 participants