react: Fold expect_callback_values into set_value

[petertseng]

react 2.0.0

expect_callback_values is often not precise enough about which
set_value calls should result in which callbacks being called or not
called. "callbacks can be added and removed" is the worst offender,
expecting a [32, 42] from callback2 only after both set_value have
fired.

That means for all we know, maybe an implementation:

• magically manages to predict the future and calls the callback twice
  on the first set_value, with the correct value, and then zero times
  on the second set_value.
• calls the callback zero times on the first set_value, but twice
  on the second set_value.

To remedy this, we observe that the only time callbacks might be called
is if there was a set_value. So, let's explicitly associate our
callback expectations along with each set_value.

[petertseng]

The necessary diff to the verify script to verify this should remain the most recent commit of https://github.com/petertseng/exercism-problem-specifications/commits/verify-react . As of the current moment, that would be petertseng@6a8b82d, but obviously I can't correct that stays correct forever, since I may need to rebase.

[petertseng]

aside: This makes it abundantly clear that there is something that has never been tested: Ensuring that a set_value that changes two output cells should in fact cause callbacks on both to be triggered. Separate PR for that (#1197)

[petertseng]

If #1197 is merged before #1196, the version number progression will be 1.2.0 -> 1.3.0 -> 2.0.0.
If the opposite happens, and #1196 is merged before #1197, the version number progression will be 1.2.0 -> 2.0.0 -> 2.1.0.

It does not matter to me which path is taken. In the absence of other feedback, I will arrange so that the final version is 2.0.0, but if there is a large disparity in time of Approval (in the GitHub sense) of the two PRs, I will of course take the appropriate path dictated by those Approvals.

[ErikSchierboom]

@petertseng Great improvement. Is this ready to be merged?

[petertseng]

The last reviewed commit (inspect the commit designated by "View changes") was 253ec11. On my rebase, the diff-of-diff is as follows:

diff -u <(git diff -u 253ec11b50ac41a9ddf457cb4eefc0cd5fd314b2~..253ec11b50ac41a9ddf457cb4eefc0cd5fd314b2) <(git diff -u react~1..react)

--- /proc/self/fd/11	2018-04-13 07:19:14.784913940 +0000
+++ /proc/self/fd/12	2018-04-13 07:19:14.786913896 +0000
@@ -1,11 +1,11 @@
 diff --git a/exercises/react/canonical-data.json b/exercises/react/canonical-data.json
-index 8774bacd..3312fc3b 100644
+index 2803aaa0..57240595 100644
 --- a/exercises/react/canonical-data.json
 +++ b/exercises/react/canonical-data.json
 @@ -1,6 +1,6 @@
  {
    "exercise": "react",
--  "version": "1.2.0",
+-  "version": "1.3.0",
 +  "version": "2.0.0",
    "comments": [
      "Note that, due to the nature of this exercise,",
@@ -102,7 +102,30 @@
            }
          ]
        },
-@@ -392,7 +381,11 @@
+@@ -398,17 +387,11 @@
+           {
+             "type": "set_value",
+             "cell": "input",
+-            "value": 10
+-          },
+-          {
+-            "type": "expect_callback_values",
+-            "callback": "callback1",
+-            "values": [11]
+-          },
+-          {
+-            "type": "expect_callback_values",
+-            "callback": "callback2",
+-            "values": [9]
++            "value": 10,
++            "expect_callbacks": {
++              "callback1": 11,
++              "callback2": 9
++            }
+           }
+         ]
+       },
+@@ -445,7 +428,11 @@
            {
              "type": "set_value",
              "cell": "input",
@@ -115,7 +138,7 @@
            },
            {
              "type": "remove_callback",
-@@ -407,22 +400,12 @@
+@@ -460,22 +447,12 @@
            {
              "type": "set_value",
              "cell": "input",
@@ -144,7 +167,7 @@
            }
          ]
        },
-@@ -479,17 +462,11 @@
+@@ -532,17 +509,11 @@
            {
              "type": "set_value",
              "cell": "input",
@@ -167,7 +190,7 @@
            }
          ]
        },
-@@ -543,12 +520,10 @@
+@@ -596,12 +567,10 @@
            {
              "type": "set_value",
              "cell": "input",
@@ -184,7 +207,7 @@
            }
          ]
        },
-@@ -597,27 +572,26 @@
+@@ -650,27 +619,26 @@
            {
              "type": "set_value",
              "cell": "input",

As you can see, expect_callbacks had to be added to the test case added in #1197.

Further, I can document that petertseng@1f62396 successfully verifies this file; the necessary documentation is the Travis CI build https://travis-ci.org/petertseng/exercism-problem-specifications/builds/365978540.

Therefore, I treat the last Approval as authoritative, and therefore that I can merge this.

Is this ready to be merged?

Yes. Thank you for reviewing!

[petertseng]

For those wondering if any test suites will actually need to change: Mostly not; the two ways of expressing callback expectations are isomorphic. However, my hope is that this way makes the intent clearer to implementors of this exercise.

However, I did sneak in one functional change, and that is to "callbacks can be added and removed", as the PR description states. For those looking to go from 1.3.0 to 2.0.0, look at what changes in that function (explicit expectations after each set_value, instead of only after the second).

It was not the wisest to combine the two operations (The transformation which changes no functionality, and one functionality change) into one commit, so I apologise for that.