Allergies: Modified canonical description to make it generatable

[macta]

The canonical-data for allergies wasn't in a format consistent with other exercises, and relied on a comment that explained that some of the expected results were actually an array of answers with the expectation that each one was a separate test. This is not possible to detect automatically and differentiate it from tests that expect array results. (I can only assume that other tracks which generate have hand edited their results).

Looking at how all the current tracks have interpreted the spec, they have all specified an "allergicToItem: 'eggs' forScore: 1" type of approach... so I propose modifying the json use sub cases to specify each one individually (this is done for lots of other exercises already).

I think this is the simplest approach - otherwise we need to adjust the schema to specify that a result can be multi-valued, and probably should have a different name like:

"manyExpected" : [ {...}, {...} ]

[macta]

realised I didn't factor this properly, will fix

[cmccandless]

To me, this doesn't seem much simpler. We're still dealing with inconsistently-nested cases fields. Perhaps something like the following would be better?

• For the allergicTo tests, tests are grouped by allergy item (all tests are nested at the same depth)
• Each group contains the following sub-cases:
  • Score is 0 (the "nothing" case) (expected: false)
  • Score is current item value (i.e. 1 for eggs, 2 for peanuts, etc) (expected: true)
  • Score is some value that is allergic to current item, but is neither the item value or 255 (expected: true)
  • Score is some value that is not allergic to current item, and is not 0 (expected: false)
  • Score is 255 (the "everything" case) (expected: true)

Example for eggs

// Top-level cases field
"cases": [
  {
    "description": "testing for egg allergy",
    "cases": [
      {
        "description": "allergic to nothing",
        "property": "allergicTo",
        "input": {
        "item": "eggs",
        "score": 0
        },
        "expected": false
      },
      {
        "description": "allergic only to eggs",
        "property": "allergicTo",
        "input": {
        "item": "eggs",
        "score": 1
        },
        "expected": true
      },
      {
        "description": "allergic to eggs and something else",
        "property": "allergicTo",
        "input": {
        "item": "eggs",
        "score": 7
        },
        "expected": true
      },
      {
        "description": "allergic something, but not eggs",
        "property": "allergicTo",
        "input": {
        "item": "eggs",
        "score": 6
        },
        "expected": false
      },
      {
        "description": "allergic to everything",
        "property": "allergicTo",
        "input": {
        "item": "eggs",
        "score": 255
        },
        "expected": true
      }
    ]
  }
]

The final test group would be the group of "list when:" tests as it currently stands now.

[macta]

@cmccandless I had limited time to spend on it - I made it better but not perfect. It's not inconsistent data wise with my suggestion (I can generate proper code from it, whereas the current version makes little sense mechanically), but I agree with you that with some thought we can make it read properly and also make it logically more consistent.

If you can run with it from here - i would be fine with that.

[petertseng]

The premise of this PR is supported by the fact that this is one of the few exercises with a custom accept in its verifier:

https://github.com/petertseng/exercism-problem-specifications/blob/b9478d45f1287da7b00fac51a722de7e38bbeaf6/exercises/allergies/verify.rb#L13

Yes, as we can see, it was necessary to interpret expected in a different way, by treating each element of expected as its own separate test (as has been pointed out). As far as I know, no other canonical-data.json does this.

The new set of tests is interesting. It is much more thorough and consistent across each allergen. Obviously there is the question of how do we make sure that we tested the right things. I think I managed to test it, by checking against implementations that, for each allergen:

1. Give correct result for an allergen
2. Always say the person is allergic to the allergen (well, unless they're allergic to nothing, since the way I implemented it 0 bitwise-and with anything is still 0).
3. Always say the person is not allergic to the allergen

The correct implementation passes all tests and the incorrect implementations each do not pass at least one test for their respective allergen, so this is good.

My Approval means I think this file is correct. This Approval will not say anything about any other considerations (whether this is a good idea for tracks) since I have no opinion on that, so people who want to say or hear such considerations should react appropriately.

This Approval is conditional on at least the first four commits being squashed together and the last two commits being squashed together, since none of those commits stand alone (they are either incorrect or do not fit the schema). Squashing all into one commit is acceptable too. Reminder that when squashing, all the lines beginning with "fix..." should be omitted from the final commit message since they are better considered a part of the commit they fix rather than a separate history entry.

The code used to test

require 'json'
require_relative '../../verify'

# Wow, this verifies the description too!
ALLERGENS = File.readlines(File.join(__dir__, 'description.md')).grep(/^\* [a-z]+ \([\d]+\)$/).map { |line|
  [line[/[a-z]+/], line[/\d+/].to_i]
}.to_h.freeze

json = JSON.parse(File.read(File.join(__dir__, 'canonical-data.json')))

raise "Have #{json['cases'].size} cases, but expected #{ALLERGENS.size + 1}" if json['cases'].size != ALLERGENS.size + 1

json['cases'][0..-2].zip(ALLERGENS.keys) { |c, allergen|
  multi_verify(c['cases'], property: 'allergicTo', implementations: {
    correct: ALLERGENS,
    always_false: ALLERGENS.merge(allergen => 0),
    always_true: ALLERGENS.merge(allergen => ~0),
  }.map { |name, allergens|
    {
      name: name,
      should_fail: name != :correct,
      f: ->(i, _) {
        i['score'] & allergens[i['item']] != 0
      },
    }
  })
}

verify(json['cases'][-1]['cases'], property: 'list') { |i, _|
  ALLERGENS.select { |_, v| i['score'] & v != 0 }.keys
}

[ErikSchierboom]

I really like this change, as previously it was one of the harder canonical data cases to auto-generate a test suite from. The new structure makes it very easy.

I second @petertseng's comment on the squashing.

[cmccandless]

As this has 2 approvals, and all comments have been resolved, I will go ahead and squash-merge this.