Skip to content

Commit

Permalink
Merge pull request #179 from stjohnjohnson/SimplifyNodeCalls
Browse files Browse the repository at this point in the history 
Cleaning up Node calls to make only one call for the computer item
  • Loading branch information
@arangamani
arangamani committed May 29, 2015
2 parents ced258a + bce5425 commit 46605b8
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 46605b8

Please sign in to comment.