dominoes: Add JSON test data

[petertseng]

Closes https://github.com/exercism/todo/issues/87

[petertseng]

Would like to verify with code before saying OK to merge.

[petertseng]

I am relatively satisfied that the JSON file is correct.

require 'json'

def chain(dominoes)
  return [true, []] if dominoes.empty?
  first = dominoes.shift
  success, subchain = chain_with(dominoes, start_with: first.last, end_with: first.first)
  [success, success && ([first] + subchain).freeze]
end

def chain_with(dominoes, start_with:, end_with:)
  return [start_with == end_with, []] if dominoes.empty?

  dominoes.each_with_index { |d, i|
    remaining_dominoes = dominoes.dup
    raise if d != remaining_dominoes.delete_at(i)

    if d.first == start_with
      target = d
    elsif d.last == start_with
      target = d.rotate
    else
      next
    end

    success, subchain = chain_with(remaining_dominoes, start_with: target.last, end_with: end_with)
    return [true, ([target] + subchain).freeze] if success
  }

  [false, nil]
end

def assert_equal(a, b, message)
  raise "#{a} != #{b} #{message}" if a != b
end

def check(input, result)
  assert_equal(input.size, result.size, "#{input} => #{result} length mismatch")
  assert_equal(input.map(&:sort).sort, result.map(&:sort).sort, "#{input} => #{result} domino mismatch")
  result.zip(result.rotate).each_with_index { |((_a, b), (c, _d)), i|
    raise "#{input} => #{result} - bad chain #{b} != #{c} at #{i}" if b != c
  }
end

# Make sure that check raises when it should
should_raises = [
  # length both ways
  ->{ check([], [1]) },
  ->{ check([1], []) },
  # using the wrong dominoes
  ->{ check([[1, 1], [2, 2]], [[1, 2], [2, 1]]) },
  # adjacent dominoes don't have an equal number
  ->{ check([[1, 3], [2, 3], [3, 1]], [[1, 3], [2, 3], [3, 1]]) },
  # head and tail don't have an equal number
  ->{ check([[1, 3], [3, 3], [3, 2]], [[1, 3], [3, 3], [3, 2]]) },
]

should_raises.each_with_index { |should_raise, i|
  begin
    should_raise[]
  rescue => e
    puts "#{i} raised #{e}, OK"
  else
    raise "#{i} didn't raise"
  end
}

puts

JSON.parse(File.read('dominoes.json'))['cases'].each_with_index { |c, i|
  input = c['input']
  can_chain, result = chain(input.dup)
  want = c['can_chain']
  raise "#{c} got #{can_chain}" if can_chain != want
  check(input, result) if want
  puts "#{i}: #{input} => #{result}"
}

0 raised 0 != 1 [] => [1] length mismatch, OK
1 raised 1 != 0 [1] => [] length mismatch, OK
2 raised [[1, 1], [2, 2]] != [[1, 2], [1, 2]] [[1, 1], [2, 2]] => [[1, 2], [2, 1]] domino mismatch, OK
3 raised [[1, 3], [2, 3], [3, 1]] => [[1, 3], [2, 3], [3, 1]] - bad chain 3 != 2 at 0, OK
4 raised [[1, 3], [3, 3], [3, 2]] => [[1, 3], [3, 3], [3, 2]] - bad chain 2 != 1 at 2, OK

0: [] => []
1: [[1, 1]] => [[1, 1]]
2: [[1, 2]] => false
3: [[1, 2], [3, 1], [2, 3]] => [[1, 2], [2, 3], [3, 1]]
4: [[1, 2], [1, 3], [2, 3]] => [[1, 2], [2, 3], [3, 1]]
5: [[1, 2], [4, 1], [2, 3]] => false
6: [[1, 1], [2, 2]] => false
7: [[1, 2], [2, 1], [3, 4], [4, 3]] => false
8: [[1, 2], [2, 3], [3, 1], [4, 4]] => false
9: [[1, 2], [2, 3], [3, 1], [2, 4], [2, 4]] => [[1, 2], [2, 4], [4, 2], [2, 3], [3, 1]]
10: [[1, 2], [2, 3], [3, 1], [1, 1], [2, 2], [3, 3]] => [[1, 2], [2, 2], [2, 3], [3, 3], [3, 1], [1, 1]]
11: [[1, 2], [5, 3], [3, 1], [1, 2], [2, 4], [1, 6], [2, 3], [3, 4], [5, 6]] => [[1, 2], [2, 1], [1, 3], [3, 2], [2, 4], [4, 3], [3, 5], [5, 6], [6, 1]]

[petertseng]

Oh, should have thought of this one... run against the rust tests grep -o '([0-9][0-9(), ]*[0-9])' dominoes.rs | tr '()' '[]', then compare the output against the json file, should all be the same.

[kytrinyx]

Niiice. OK, so it sounds like you're happy with this one? Has someone else been involved in the dominoes problem at all (@IanWhitney, maybe)?

[IanWhitney]

I've scanned the exercise, but haven't yet done it myself.

[kytrinyx]

Same here. This looks good to me, but I don't know enough to catch anything interesting. so... LGTM.

[petertseng]

I know that @masters3d has done the exercise (on account of @masters3d having ported it to Swift), so maybe would have some opinions

[masters3d]

What do you mean by second level of testing? Otherwise looks good. I mirrored the test cases from the rust track when I implemented in swift and the Json looks the same.

[petertseng]

The former:

• Exercise asks the question "Can a chain be made?"
• Student's code only needs to return a boolean value
• Test code only checks whether the boolean value matches up.

The latter:

• Exercise asks the question "Can a chain be made? And if it can, what is it?"
• Student's code needs to return Option<Vector<Domino>> (or the equivalent in the language)
• Test code needs to check the four properties listed in the comments of the JSON document.

[petertseng]

I would like to edit the JSON to highlight the difference between the boolean value versus the Option<Vector<Domino>>

[masters3d]

Ah, that makes it clear. Wouldn't the second level of testing leak out the implementation/solution within the test code?

[petertseng]

Along with that question, I realized there are three related questions.

Does it unnecessarily constrain the types?

The test code had to define a Domino type to pass into the function-under-test as well, so it is not unreasonable to expect that the function-under-test also returns an optional list of that same Domino type that is already in the input.

Does it reveal how to solve the exercise?

A test that merely checks some properties of a correct solution does not necessarily reveal how to achieve such a property. The Rust test code is quite different from the Rust example solution, and so too the Ruby test code is different form the Ruby example solution.

If the question was of the form "Give me two numbers other than (1, 51) that when multiplied give me 51" the test code might multiply the two numbers given and see that their product is indeed equal to 51 - that wouldn't reveal anything about how someone might come up with the two numbers.

But someone could just brute force it using the verification code!

OK, that's true. The approach would be: Take the verification code in the test solution, run it against all permutations of the input. Return the first permutation that passes the verification code.

I think that someone trying to use such a solution might be stymied by the fact that dominoes can be reversed. It will be difficult to make such a solution work because of that. But someone might still try. I accept the risk that putting verification code in the test suite will make their lives easier, because I'm honestly a bit curious to see how such a solution deals with reversing dominoes in permutations.

In general, a lot of time this is up to the honor system. In the leap year exercise, we ask students to do it without using standard libraries, and in some tracks maybe we check that they don't use the standard library: https://github.com/exercism/xruby/blob/master/exercises/leap/leap_test.rb#L11

But eventually there comes a point where we can't foresee and prevent everything a student might do. In the accumulate exercise Ruby no longer checks that the student isn't using map or collect: https://github.com/exercism/xruby/blob/master/exercises/accumulate/accumulate_test.rb for example. And if someone did, they're only cheating themselves.

Can it be exhaustive?

We'll have to accept that there may be some solutions that may pass the tests but are still incorrect. I've seen some submissions that were able to give a correct solution to [[1, 2], [2, 3], [3, 1], [2, 4], [2, 4]] and gave an incorrect solution to [[2, 4], [2, 4], [1, 2], [2, 3], [3, 1]], so they were dependent on the ordering of the input! Can we possibly test all orderings of the input? Probably not, and some review will necessarily have to be left to reviewers on the site. Tests can't prove the absence of bugs, unfortunately.

[masters3d]

cool, that works. The domino chain exercise was a little challenging for me; I got bit by ordering of input with it.

[petertseng]

Thanks for talking through and for pointing out where the comment was unclear. I'm updating with:

@@ -5,8 +5,16 @@
       "For example, if the target language has 2-tuples, that is a good candidate.",
       "",
       "There are two levels of this exercise that can be implemented and/or tested:",
+      "",
       "1: Given a list of dominoes, determine whether it can be made into a chain.",
+      "Under this scheme, the submitted code only needs to return a boolean.",
+      "The test code only needs to check that that boolean value matches up.",
+      "",
       "2: Given a list of dominoes, determine one possible chain, if one exists, or else conclude that none can be made.",
+      "Under this scheme, the submitted code needs to either return a chain, or signal that none exists.",
+      "Different languages may do this differently:",
+      "return Option<Vector<Domino>>, return ([]Domino, error), raise exception, etc.",
+      "The test code needs to check that the returned chain is correct (see below).",
       "",
       "It's infeasible to list every single possible result chain in this file.",
       "That's because for even a simple list [(1, 2), (2, 3), (3, 1)],",

Also an unrelated change, to make clear where there are dominoes not explicitly listed:

@@ -19,8 +27,8 @@
       "The properties to verify are:",
       "1. The submitted code claims there is a chain if and only if there actually is one.",
       "2. The number of dominoes in the output equals the number of dominoes in the input.",
-      "3a. For each adjacent pair of dominoes (a, b), (c, d): b is equal to c.",
-      "3b. For the dominoes on the ends (a, b), ... (c, d): a is equal to d.",
+      "3a. For each adjacent pair of dominoes ... (a, b), (c, d) ...: b is equal to c.",
+      "3b. For the dominoes on the ends (a, b) ... (c, d): a is equal to d.",
       "4. Every domino appears in the output an equal number of times as the number of times it appears in the input.",
       "(in other words, the dominoes in the output are the same dominoes as the ones in the input)",
       "",

[petertseng]

(and can you believe I posted that diff and subsequently forgot to push it... it is pushed)

ready to go now.

[IanWhitney]

Looks good to me.

[kytrinyx]

All this time I thought all track maintainers had commit on this repo... and I had given everyone "read" access. Sheesh. This should be fixed now, merge at your discretion (I merged this one) :)