Skip to content

Commit

Permalink
Cleaning up Node calls to make only one call for the computer item th…
Browse files Browse the repository at this point in the history 
…ey are looking for
  • Loading branch information
@stjohnjohnson
stjohnjohnson committed May 29, 2015
1 parent ced258a commit bce5425
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 42 deletions.
14 changes: 8 additions & 6 deletions lib/jenkins_api_client/node.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ def index(node_name)
GENERAL_ATTRIBUTES.each do |meth_suffix|
define_method("get_#{meth_suffix}") do
@logger.info "Obtaining '#{meth_suffix}' attribute from jenkins"
response_json = @client.api_get_request("/computer")
response_json = @client.api_get_request("/computer", "tree=#{path_encode meth_suffix}")
response_json["#{meth_suffix}"]
end
end
Expand All @@ -254,8 +254,9 @@ def index(node_name)
NODE_PROPERTIES.each do |meth_suffix|
define_method("is_#{meth_suffix}?") do |node_name|
@logger.info "Obtaining '#{meth_suffix}' property of '#{node_name}'"
response_json = @client.api_get_request("/computer")
resp = response_json["computer"][index(node_name)]["#{meth_suffix}"].to_s
node_name = "(master)" if node_name == "master"
response_json = @client.api_get_request("/computer/#{path_encode node_name}", "tree=#{path_encode meth_suffix}")
resp = response_json["#{meth_suffix}"].to_s
resp =~ /False/i ? false : true
end
end
Expand All @@ -264,8 +265,9 @@ def index(node_name)
NODE_ATTRIBUTES.each do |meth_suffix|
define_method("get_node_#{meth_suffix}") do |node_name|
@logger.info "Obtaining '#{meth_suffix}' attribute of '#{node_name}'"
response_json = @client.api_get_request("/computer")
response_json["computer"][index(node_name)]["#{meth_suffix}"]
node_name = "(master)" if node_name == "master"
response_json = @client.api_get_request("/computer/#{path_encode node_name}", "tree=#{path_encode meth_suffix}")
response_json["#{meth_suffix}"]
end
end

Expand All @@ -292,7 +294,7 @@ def change_mode(node_name, mode)
def get_config(node_name)
@logger.info "Obtaining the config.xml of node '#{node_name}'"
node_name = "(master)" if node_name == "master"
@client.get_config("/computer/#{ path_encode node_name}")
@client.get_config("/computer/#{path_encode node_name}")
end

# Posts the given config.xml to the Jenkins node
Expand Down
111 changes: 75 additions & 36 deletions spec/unit_tests/node_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,36 +7,31 @@
mock_logger = Logger.new "/dev/null"
@client.should_receive(:logger).and_return(mock_logger)
@node = JenkinsApi::Client::Node.new(@client)
@sample_json_computer_response = {
@sample_json_list_response = {
"computer" => [
"displayName" => "slave"
]
}
@sample_json_computer_response = {
"displayName" => "slave"
}
@offline_slave = {
"computer" => [
"displayName" => "slave",
"offline" => true,
"temporarilyOffline" => true,
]
"displayName" => "slave",
"offline" => true,
"temporarilyOffline" => true,
}
@online_slave = {
"computer" => [
"displayName" => "slave",
"offline" => false,
"temporarilyOffline" => false,
]
"displayName" => "slave",
"offline" => false,
"temporarilyOffline" => false,
}
@offline_slave_in_string = {
"computer" => [
"displayName" => "slave",
"offline" => "true",
]
"displayName" => "slave",
"offline" => "true",
}
@online_slave_in_string = {
"computer" => [
"displayName" => "slave",
"offline" => "false",
]
"displayName" => "slave",
"offline" => "false",
}
computer_sample_xml_filename = '../fixtures/files/computer_sample.xml'
@sample_computer_xml = File.read(
Expand Down Expand Up @@ -117,7 +112,7 @@
).with(
"/computer"
).and_return(
@sample_json_computer_response
@sample_json_list_response
)
@client.should_receive(
:api_post_request
Expand All @@ -135,7 +130,7 @@
).with(
"/computer"
).and_return(
@sample_json_computer_response
@sample_json_list_response
)
expect(
lambda{ @node.delete(slave_name) }
Expand All @@ -147,8 +142,10 @@
it "accepts filter and lists all nodes matching the filter" do
@client.should_receive(
:api_get_request
).with(
"/computer"
).and_return(
@sample_json_computer_response
@sample_json_list_response
)
@node.list("slave").class.should == Array
end
Expand All @@ -161,8 +158,11 @@
it "should get the #{attribute} attribute" do
@client.should_receive(
:api_get_request
).with(
"/computer",
"tree=#{attribute}"
).and_return(
@sample_json_computer_response
@sample_json_list_response
)
@node.method("get_#{attribute}").call
end
Expand All @@ -177,11 +177,26 @@
it "should get the #{property} property" do
@client.should_receive(
:api_get_request
).twice.and_return(
).with(
"/computer/slave",
"tree=#{property}"
).and_return(
@sample_json_computer_response
)
@node.method("is_#{property}?").call("slave")
end

it "should get the #{property} property for master" do
@client.should_receive(
:api_get_request
).with(
"/computer/(master)",
"tree=#{property}"
).and_return(
@sample_json_computer_response
)
@node.method("is_#{property}?").call("master")
end
end
end
end
Expand All @@ -190,7 +205,10 @@
it "returns true if the node is offline" do
@client.should_receive(
:api_get_request
).twice.and_return(
).with(
"/computer/slave",
"tree=offline"
).and_return(
@offline_slave
)
@node.method("is_offline?").call("slave").should be_true
Expand All @@ -199,7 +217,10 @@
it "returns false if the node is online" do
@client.should_receive(
:api_get_request
).twice.and_return(
).with(
"/computer/slave",
"tree=offline"
).and_return(
@online_slave
)
@node.method("is_offline?").call("slave").should be_false
Expand All @@ -208,7 +229,10 @@
it "returns false if the node is online and have a string value on its attr" do
@client.should_receive(
:api_get_request
).twice.and_return(
).with(
"/computer/slave",
"tree=offline"
).and_return(
@offline_slave_in_string
)
@node.method("is_offline?").call("slave").should be_true
Expand All @@ -217,7 +241,10 @@
it "returns false if the node is online and have a string value on its attr" do
@client.should_receive(
:api_get_request
).twice.and_return(
).with(
"/computer/slave",
"tree=offline"
).and_return(
@online_slave_in_string
)
@node.method("is_offline?").call("slave").should be_false
Expand All @@ -231,11 +258,26 @@
it "should get the #{attribute} node attribute" do
@client.should_receive(
:api_get_request
).twice.and_return(
).with(
"/computer/slave",
"tree=#{attribute}"
).and_return(
@sample_json_computer_response
)
@node.method("get_node_#{attribute}").call("slave")
end

it "should get the #{attribute} node attribute for master" do
@client.should_receive(
:api_get_request
).with(
"/computer/(master)",
"tree=#{attribute}"
).and_return(
@sample_json_computer_response
)
@node.method("get_node_#{attribute}").call("master")
end
end
end
end
Expand Down Expand Up @@ -266,12 +308,10 @@
@client.should_receive(
:api_get_request
).with(
"/computer"
"/computer/slave",
"tree=temporarilyOffline"
).and_return(
@offline_slave,
@offline_slave,
# Note: each of these is_? requires two API calls
@online_slave,
@online_slave
)
@node.method("toggle_temporarilyOffline").call("slave", "foo bar").should be_false
Expand All @@ -284,10 +324,9 @@
@client.should_receive(
:api_get_request
).with(
"/computer"
"/computer/slave",
"tree=temporarilyOffline"
).and_return(
@online_slave,
@online_slave,
@online_slave,
@online_slave
)
Expand Down

0 comments on commit bce5425

Please sign in to comment.