aeolusproject / conductor

Aeolus Conductor
http://www.aeolusproject.org/
Apache License 2.0
32 stars 27 forks source link

Refactor Provider Account priority #432

Closed ifarkas closed 11 years ago

ifarkas commented 11 years ago

This patchset includes the following changes:

martinpovolny commented 11 years ago

I would shorten this function, too verbose for my feeling of "pretty"

+Then /^I should see score "(.*?)" for provider account with label "(.*?)"$/ do |score, provider_account_label|
+  found = false
+  all("tr").each do |tr|
+    if tr.has_content?(provider_account_label) && tr.has_content?(score)
+      found = true
+    end
+  end
+  assert found
+end

step #1:

found = false                                                                   
all("tr").each do |tr|                                                          
  found ||= tr.has_content?(provider_account_label) && tr.has_content?(score)   
end                                                                             
assert found     

step #2:

assert all("tr").detect { |tr| tr.has_content?(provider_account_label) && tr.has_content?(score) }

over all looks good, works, tests pass

ACK and merging it, just consider revisiting the function in question, please

ifarkas commented 11 years ago

@martinpovolny, sure, I will create a new pull request fixing that. Thanks for the review!