0

The problem is: I have a method

def comparison_reporter(list_of_scenarios_results1, list_of_scenarios_results2)
  actual_failed_tests = list_of_scenarios_results2.select {|k,v| v == 'Failed'}
  actual_passed_tests = list_of_scenarios_results2.select {|k,v| v == 'Passed'}
  failed_tests = Array.new(actual_failed_tests.length) { Hash.new }
  failed_tests.each do |hash|
      actual_failed_tests.keys.map {|name| hash["test_name"] = name}
      actual_failed_tests.values.map {|new_status| hash["actual_status"] = new_status}
      list_of_scenarios_results1.values_at(*actual_failed_tests.keys).map {|old_status| hash["previous_status"] = old_status}
  end
  final_result = {
    "passed_tests_count" => list_of_scenarios_results2.select {|k,v| v == 'Passed'}.length,
    "failed_tests_count" => list_of_scenarios_results2.select {|k,v| v == 'Failed'}.length,
    "failed_tests" => failed_tests
  }
  return final_result
end

This method takes 2 hashes as arguments and returns the result of their comparison and some other things. Currently, it always returns failed_tests with two (or more) identical hashes (same key-value pairs). I think, that problem is somewhere in failed_tests.each do |hash| block, but I can't find the reason of this bug, please advice. Example of the method result (in .json format)

{
  "passed_tests_count": 3,
  "failed_tests_count": 2,
  "failed_tests": [
    {
      "test_name": "As a user I want to use Recent searches tab",
      "actual_status": "Failed",
      "previous_status": "Failed"
    },
    {
      "test_name": "As a user I want to use Recent searches tab",
      "actual_status": "Failed",
      "previous_status": "Failed"
    }
  ]
}

UPD: hash1 (first argument) -

{""=>"Passed",
"As a new user I want to use no fee rentals tab"=>"Passed",
"As a new user I want to use Luxury rentals tab"=>"Passed", 
"As a user I want to use Recent searches tab"=>"Failed",
"As a user I want to use new listings for you tab"=>"Passed"}

hash2 (second argument)-

{""=>"Passed",
"As a new user I want to use no fee rentals tab"=>"Failed",
"As a new user I want to use Luxury rentals tab"=>"Passed",
"As a user I want to use Recent searches tab"=>"Failed",
"As a user I want to use new listings for you tab"=>"Passed"}

Example of desired desired output:

{ 
"passed":"count",
"failed":"count",
"failed_tests": [
   {"test_name":"As a user I want to use Recent searches tab",
    "actual_status":"failed",
    "previous_status":"failed"},
   {"test_name":"As a new user I want to use no fee rentals tab",
    "actual_status":"failed",
    "previous_status":"passed"}]
}
Marcin Kołodziej
  • 5,253
  • 1
  • 10
  • 17
Mikhah
  • 139
  • 12
  • What is the problem that you have in somewhere in `failed_tests.each do |hash|` block? You haven't stated it. What do you expect the method to do? – sawa Nov 28 '18 at 13:26
  • Yep, sorry. The method should return `final_result` which should also contain `failed_tests` array with 2 different hashes (results and comparison for different tests). Currently, as you can see from the example - both hashes are actually the same, therefore I thought that smthn is wrong in `failed_tests.each do |hash|` – Mikhah Nov 28 '18 at 13:38
  • 1
    @Mikhah it's nice that you showed effort, but it's difficult to read through your method. If you want an answer, provide your input (the 2 hashes) and the hash you'd like to be the result. – Marcin Kołodziej Nov 28 '18 at 14:15
  • @sawa ofc, I've updated the question – Mikhah Nov 28 '18 at 14:27
  • @MarcinKołodziej sorry, missclicked) I've updated the question – Mikhah Nov 28 '18 at 14:46

2 Answers2

2

Solution:

def comparison_reporter(before, after)
  failed_tests = after.select { |k,v| v == "Failed" }.map do |k,v|
    {
      test_name: k,
      actual_status: v,
      previous_status: before[k]
    }
  end

  {
    passed: after.size - failed_tests.size,
    failed: failed_tests.size,
    failed_tests: failed_tests
  }
end

Simplified failed_tests quite a bit. Since we calculate number of failed tests, we can use it for the final counts, instead of iterating over the hash again.

Marcin Kołodziej
  • 5,253
  • 1
  • 10
  • 17
  • Thanks for the simplification and answer, it works perfectly) – Mikhah Nov 28 '18 at 16:03
  • 1
    Perhaps `...after.each_with_object([]) { |(k,v),a| a << { test_name: k,actual_status: v, previous_status: before[k] } if v=="Failed" }` to traverse the array just once. – Cary Swoveland Nov 28 '18 at 19:32
1

The problem is on line 8: You're overwriting hash["previous_status"] with the last value in list_of_scenarios_results1.values_at(*actual_failed_tests.keys) when you map over it.

Usually you use map to assign an iterable to something, not modify something else. e.g.
x = ['1','2','3'].map(&:to_i)
rather than
x = []; ['1','2','3'].map {|v| x << v.to_i}

I'd suggest re-thinking your approach. Will you always have the same keys in both hashes? If so you could simplify this. I'd also suggest looking into byebug. It's an interactive debugger that'll let you step through your function and see where things aren't doing what you expect.

ConorSheehan1
  • 1,640
  • 1
  • 18
  • 30