Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

Update to modern RSpec #976

Merged
merged 1 commit into from
Feb 10, 2017
Merged

Update to modern RSpec #976

merged 1 commit into from
Feb 10, 2017

Conversation

twalpole
Copy link
Collaborator

@twalpole twalpole commented Feb 4, 2017

This updates the tests to expect syntax, and fixes all warnings/deprecation when using RSpec 3.5.x. Some tests which will now pass against Capybara 2.12.0+ are also enabled to run on those versions. The tests get run with both RSpec 2.x and 3.5.x on travis.

One Capybara test that had to be disabled (passed on RSpec 2.x but failed on 3.x because the block passed to raise_error was ignored when RSpec 2.x ran the test). The test is "Capybara::Session webkit node #reload without automatic reload should not automatically reload" which fails because it returns the wrong type of error. This is caused by the fact that capybara-webkit allows access to detached nodes when Capybara.automatic_reload is false (other drivers report that the element is no longer valid). I'm not sure what the point of this behavior is, but it is intentional and is tested for in capybara-webkits tests.

"expected #{response} to include #{expected_response}"
end

failure_message_for_should_not do |actual|
send(respond_to?(:failure_message_when_negated) ? :failure_message_when_negated : :failure_message_for_should_not) do |actual|

Choose a reason for hiding this comment

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

Line is too long. [128/80]
Unused block argument - actual. You can omit the argument if you don't care about it.

@@ -14,11 +14,11 @@
found_response
end

failure_message_for_should do |actual|
send(respond_to?(:failure_message) ? :failure_message : :failure_message_for_should) do |actual|

Choose a reason for hiding this comment

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

Line is too long. [98/80]
Unused block argument - actual. You can omit the argument if you don't care about it.

description =~ /Capybara::Session webkit Capybara::Window\s*#(size|resize_to|maximize|close.*no_such_window_error|send_keys)/ ||
description =~ /Capybara::Session webkit node\s*#set should allow me to change the contents of a contenteditable elements child/
else
description =~ /Capybara::Session webkit Capybara::Window\s*#close.*no_such_window_error/

Choose a reason for hiding this comment

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

Line is too long. [97/80]

description =~ /Capybara::Session webkit node #reload without automatic reload should not automatically reload/ ||
if Gem::Version.new(Capybara::VERSION) < Gem::Version.new("2.12.0")
description =~ /Capybara::Session webkit Capybara::Window\s*#(size|resize_to|maximize|close.*no_such_window_error|send_keys)/ ||
description =~ /Capybara::Session webkit node\s*#set should allow me to change the contents of a contenteditable elements child/

Choose a reason for hiding this comment

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

Line is too long. [136/80]

description =~ /Capybara::Session webkit node #send_keys/ ||
description =~ /Capybara::Session webkit node #reload without automatic reload should not automatically reload/ ||
if Gem::Version.new(Capybara::VERSION) < Gem::Version.new("2.12.0")
description =~ /Capybara::Session webkit Capybara::Window\s*#(size|resize_to|maximize|close.*no_such_window_error|send_keys)/ ||

Choose a reason for hiding this comment

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

Line is too long. [136/80]

description =~ /Capybara::Session webkit Capybara::Window #(size|resize_to|maximize|close.*no_such_window_error|send_keys)/ ||
description =~ /Capybara::Session webkit node #send_keys/
description =~ /Capybara::Session webkit node #send_keys/ ||
description =~ /Capybara::Session webkit node #reload without automatic reload should not automatically reload/ ||

Choose a reason for hiding this comment

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

Line is too long. [122/80]

# We focus the next window instead of failing when closing windows.
# We allow accessing unattached nodes when reload is disabled ???
# Node#send_keys does not support modifiers and only supports a subset of special keys

Choose a reason for hiding this comment

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

Line is too long. [88/80]

@@ -48,12 +47,20 @@ def has_internet?
require 'capybara_webkit_builder'
c.filter_run_excluding :skip_on_qt4 => !(%x(#{CapybaraWebkitBuilder.qmake_bin} -v).match(/Using Qt version 4/)).nil?

# We can't support outerWidth and outerHeight without a visible window.
# We can't support outerWidth and outerHeight without a visible window. Only affects Capybara < 2.12.0

Choose a reason for hiding this comment

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

Line is too long. [104/80]

@@ -538,7 +538,7 @@ module TestSessions
session.attach_file 'File', file.path
session.click_on 'Go'

session.should have_text('Hello')
expect(session).to have_text('Hello')

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@@ -460,7 +460,7 @@ module TestSessions

it 'raises an error if an element is not in the viewport when clicked' do
subject.visit('/')
lambda { subject.click_link "Click Me" }.should raise_error(Capybara::Webkit::ClickFailed)
expect { subject.click_link "Click Me" }.to raise_error(Capybara::Webkit::ClickFailed)

Choose a reason for hiding this comment

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

Line is too long. [92/80]

@@ -438,9 +438,9 @@ module TestSessions
document.body.appendChild(div);
JS

lambda {
expect {

Choose a reason for hiding this comment

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

Avoid using {...} for multi-line blocks.

@@ -417,11 +417,11 @@ module TestSessions
expect {
subject.find(:css, '#one').click
}.to raise_error(Capybara::Webkit::ClickFailed) { |exception|
exception.message.should =~ %r{Failed.*\[@id='one'\].*overlapping.*\[@id='two'\].*at position}
expect(exception.message).to match %r{Failed.*\[@id='one'\].*overlapping.*\[@id='two'\].*at position}

Choose a reason for hiding this comment

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

Line is too long. [109/80]

subject.find(:css, '#two')['data-click-x'].should_not be_nil
subject.find(:css, '#two')['data-click-y'].should_not be_nil
expect(subject.find(:css, '#two')['data-click-x']).not_to be_nil
expect(subject.find(:css, '#two')['data-click-y']).not_to be_nil

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

end

it 'scrolls an element into view when clicked' do
subject.visit('/')
subject.driver.resize_window(200, 200)
subject.find(:css, '#two').click
subject.find(:css, '#two')['data-click-x'].should_not be_nil
subject.find(:css, '#two')['data-click-y'].should_not be_nil
expect(subject.find(:css, '#two')['data-click-x']).not_to be_nil

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

end

it 'does not raise an error when an anchor contains empty nodes' do
subject.visit('/')
lambda { subject.click_link('Some link') }.should_not raise_error
expect { subject.click_link('Some link') }.not_to raise_error

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

subject.find(:css, '#one')['data-click-x'].should eq '149'
subject.find(:css, '#one')['data-click-y'].should eq '99'
expect(subject.find(:css, '#one')['data-click-x']).to eq '149'
expect(subject.find(:css, '#one')['data-click-y']).to eq '99'

Choose a reason for hiding this comment

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

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@jferris
Copy link
Contributor

jferris commented Feb 6, 2017

This is caused by the fact that capybara-webkit allows access to detached nodes when Capybara.automatic_reload is false (other drivers report that the element is no longer valid).

We introduced that flag for backwards compatibility (that was how capybara-webkit happened to behave before Capybara added auto-reloading).

We also had some concerns about implementing it without causing performance issues, so we left in an option to disable the DOM attachment check. I haven't run benchmarks to compare in a long time, but there was a significant performance improvement by not using the auto-reload feature. However, I've run most of my test suites using capybara-webkit and auto-reloading enabled, and I haven't had any problems.

We could likely deprecate this flag and remove it in the next major release.

# We focus the next window instead of failing when closing windows.
# Accessing unattached nodes is allowed when reload is disabled - Legacy behavior

Choose a reason for hiding this comment

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

Line is too long. [83/80]

# We focus the next window instead of failing when closing windows.
# Accessing unattached nodes is allowed when reload is disabled - Legacy behavior

Choose a reason for hiding this comment

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

Line is too long. [83/80]

@twalpole
Copy link
Collaborator Author

twalpole commented Feb 6, 2017

@jferris That explains it - updated the comment

@jferris jferris merged commit ed295f2 into thoughtbot:master Feb 10, 2017
@jferris
Copy link
Contributor

jferris commented Feb 10, 2017

Thanks! This is merged into master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants