Skip to content

Commit

Permalink
Fix: Do not expect response for initiatorType Ping requests (#417)
Browse files Browse the repository at this point in the history
* Add Request#response_expected? method

* Add Exchange#response_expected? method

* Exchange#finished? returns true if no response expected

* Add HTML test for <a> with ping attribute
  • Loading branch information
MC-Squared authored Jan 6, 2024
1 parent ec912a1 commit b5a5ae8
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 8 deletions.
13 changes: 12 additions & 1 deletion lib/ferrum/network/exchange.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ def blocked?
# @return [Boolean]
#
def finished?
blocked? || response&.loaded? || !error.nil?
blocked? || response&.loaded? || !error.nil? || !response_expected?
end

#
Expand Down Expand Up @@ -118,6 +118,17 @@ def redirect?
response&.redirect?
end

#
# Determines if the exchange expects a response
#
# @return [Boolean]
#
def response_expected?
return true if request.nil?

!!request.response_expected?
end

#
# Returns request's URL.
#
Expand Down
9 changes: 9 additions & 0 deletions lib/ferrum/network/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,15 @@ def time
@time ||= Time.strptime(@params["wallTime"].to_s, "%s")
end

#
# Determines if a response is expected.
#
# @return [Boolean]
#
def response_expected?
!type?("ping")
end

#
# Converts the request to a Hash.
#
Expand Down
26 changes: 26 additions & 0 deletions spec/network/exchange_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,22 @@

expect(last_exchange.finished?).to be true
end

it "returns true if an error occurred" do
exchange = Ferrum::Network::Exchange.new(page, "1")

expect(exchange.finished?).to be false
exchange.error = Ferrum::Network::Error.new
expect(exchange.finished?).to be true
end

it "returns true for ping requests" do
exchange = Ferrum::Network::Exchange.new(page, "1")
expect(exchange.finished?).to be false

exchange.request = Ferrum::Network::Request.new({"type" => "Ping"})
expect(exchange.finished?).to be true
end
end

describe "#redirect?" do
Expand All @@ -119,6 +135,16 @@
end
end

describe "#response_expected?" do
it "determines if exchange expects a response" do
exchange = Ferrum::Network::Exchange.new(page, "1")
expect(exchange.response_expected?).to be true

exchange.request = Ferrum::Network::Request.new({"type" => "Ping"})
expect(exchange.response_expected?).to be false
end
end

describe "#pending?" do
it "determines if exchange is not fully loaded" do
allow(page).to receive(:timeout) { 2 }
Expand Down
14 changes: 13 additions & 1 deletion spec/network/request_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
# frozen_string_literal: true

describe Ferrum::Network::Request do
skip
describe "#response_expected?" do
it "returns true for document requests" do
request = Ferrum::Network::Request.new({"type" => "Document"})

expect(request.response_expected?).to be(true)
end

it "returns false for ping requests" do
request = Ferrum::Network::Request.new({"type" => "Ping"})

expect(request.response_expected?).to be(false)
end
end
end
22 changes: 16 additions & 6 deletions spec/network_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,23 @@
expect(page.body).to include("test_cookie")
end

it "#idle?" do
page.go_to("/ferrum/with_slow_ajax_connection")
expect(page.at_xpath("//h1[text() = 'Slow AJAX']")).to be
describe "#idle?" do
it "waits for network to be idle" do
page.go_to("/ferrum/with_slow_ajax_connection")
expect(page.at_xpath("//h1[text() = 'Slow AJAX']")).to be

expect(network.idle?).to be_falsey
network.wait_for_idle
expect(network.idle?).to be_truthy
end

expect(network.idle?).to be_falsey
network.wait_for_idle
expect(network.idle?).to be_truthy
it "does not wait for responses to PING requests" do
page.go_to("/ferrum/link_with_ping")
page.at_css("a").click

network.wait_for_idle
expect(network.idle?).to be_truthy
end
end

it "#total_connections" do
Expand Down
10 changes: 10 additions & 0 deletions spec/node_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@
expect(browser.current_url).to eq(base_url("/"))
end

it "fires a ping request for anchor elements" do
browser.go_to("/ferrum/link_with_ping")

expect(browser.network.traffic.length).to eq(1)
browser.at_css("a").click

# 1 for first load, 1 for load of new url, 1 for ping = 3 total
expect(browser.network.traffic.length).to eq(3)
end

it "does not run into content quads error" do
browser.go_to("/ferrum/index")

Expand Down
6 changes: 6 additions & 0 deletions spec/support/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,12 @@ def authorized?(login, password)
params["remaining_path"]
end

post "/ferrum/ping" do
# Sleeping to simulate a server that does not send a response to PING requests
sleep 5
halt 204
end

protected

def render_view(view)
Expand Down
3 changes: 3 additions & 0 deletions spec/support/views/link_with_ping.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<p>
<a href="/host" ping="/ferrum/ping">Link with ping</a>
</p>

0 comments on commit b5a5ae8

Please sign in to comment.