kindergarten-garden: add canonical data #770

holdenout · 2017-05-02T21:41:04Z

Closes #565

Notes:

The default students indicated in the README are assumed, and alternate students are indicated as input when appropriate.
While some tracks tested all 12 students for the full garden, it only seemed necessary to include a few tests here.

petertseng

thanks!

petertseng · 2017-05-02T21:47:41Z

exercises/kindergarten-garden/canonical-data.json

+          "description": "test Patricia's garden",
+          "property": "plants",
+          "students": ["Samantha", "Patricia", "Xander", "Roger"],
+          "diagram": ["VCRRGVRG\nRVGCCGCV"],


array -> string? None of the diagram arrays have more than one element

petertseng · 2017-05-02T21:50:23Z

exercises/kindergarten-garden/canonical-data.json

+            {
+              "description": "test Bob's garden",
+              "diagram": "VVCCGG\nVVCCGG",
+              "property": "plants",


anyone have a desire for consistent ordering? notice that property comes after diagram here, but before diagram in above cases

(has since been resolved)

petertseng · 2017-05-02T21:51:52Z

exercises/kindergarten-garden/canonical-data.json

+          "expected": ["grass", "clover", "clover", "grass"]
+        },
+        {
+          "description": "test Larry's garden",


it might be useful to say in the description that Larry is the last student with plants in this garden (is that right? I am guessing)

petertseng · 2017-05-02T21:52:34Z

exercises/kindergarten-garden/canonical-data.json

+      ]
+    },
+    {
+      "description": "disordered tests",


I'm unclear on what is disordered - I think "tests of disordered X" would help readers

I believe it is supposed to be the names. I'll update that description. The README doesn't specify this, but it seems that that the students gardens are in alphabetical order by name. Is that something that should be added to the README?

petertseng · 2017-05-02T21:54:58Z

exercises/kindergarten-garden/canonical-data.json

+      "description": "partial garden tests",
+      "cases": [
+        {
+          "description": "test Alice's garden",


I don't know if it's just me, but it seemed more important to me to say that this tests a garden with only a single student, rather than stating the name of the student (already inferrable from "student": key)

That definitely makes sense so it would explain the reasoning behind the test rather than just what it's doing. I'll fix that along with the similar ones below.

petertseng · 2017-05-02T21:55:16Z

exercises/kindergarten-garden/canonical-data.json

+          "expected": ["violets", "clover", "radishes", "clover"]
+        },
+        {
+          "description": "test Bob's garden",


in the same vein, it seems more valuable to me to say that now we're testing a garden with a second student

petertseng · 2017-05-02T21:56:45Z

exercises/kindergarten-garden/canonical-data.json

+          "expected": ["clover", "grass", "radishes", "clover"]
+        },
+        {
+          "description": "test Bob and Charlie's garden",


here too - three students.

holdenout · 2017-05-02T22:44:33Z

Thanks for the feedback! I'll get right on those. Should I just push the new commits, and squash them when it's all approved?

hilary · 2017-05-02T22:45:40Z

@holdenout yes, please push the new commits, then squash away 😄

petertseng

Verified with:

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

DEFAULT_NAMES = %w(Alice Bob Charlie David Eve Fred Ginny Harriet Ileana Joseph Kincaid Larry).map(&:freeze).freeze
PLANTS = {
  ?C => 'clover',
  ?G => 'grass',
  ?R => 'radishes',
  ?V => 'violets',
}.freeze
PLANTS_PER_ROW_PER_KID = 2

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

cases = json['cases'].flat_map { |c| c['cases'].flat_map { |cc| cc['cases'] || cc } }

verify(cases, property: 'plants') { |c|
  kid_names = (c['students'] || DEFAULT_NAMES).sort

  rows = c['diagram'].split.map { |row|
    row.each_char.each_slice(PLANTS_PER_ROW_PER_KID)
  }
  plant_groups = rows[0].zip(*rows[1..-1]).map { |group_rows|
    group_rows.flatten.map(&PLANTS).freeze
  }

  plant_groups[kid_names.index(c['student'])]
}

All passed.

Okay, I think I have just a few small things to note now.

petertseng · 2017-05-03T06:27:17Z

exercises/kindergarten-garden/canonical-data.json

+    "Garden can be created with just a diagram or a diagram and students.",
+    "Students' gardens are assigned in alphabetical order.",
+    "The default students are:",
+    "  Alice, Bob, Charlie, David, Eve, Fred, Ginny, Harriet, Ileana, Joseph, Kincaid, and Larry."


since this information is also in the https://github.com/exercism/x-common/blob/master/exercises/kindergarten-garden/description.md, any chance of it falling out date? I try to reduce duplication when possible. I am thinking it doesn't need to be put here.

Ah, that does make sense. Were you referring to all 4 lines? I included the bit about the default students because I thought it might help clarify some of the issues brought up in #659. Would that still be considered unnecessary though?

Ah, I was meaning to refer only to the list of the twelve default students...

But now that you bring it up, yes, "Students' gardens are assigned in alphabetical order." is also mentioned in the description! So I would also think that should be removed.

If there is to be clarification for #659, I would suggest it go in the description since the README is where students will likely go to look for instructions for how to do this exercise. It wouldn't be visible to students if it were placed in this canonical-data.json file. Since this PR is just dealing with the canonical data, we don't need to deal with this now, unless you would like to add another commit to this PR or a separate PR.

petertseng · 2017-05-03T06:42:11Z

exercises/kindergarten-garden/canonical-data.json

+      "description": "partial garden tests",
+      "cases": [
+        {
+          "description": "test garden with single student",


at this point, all these descriptions begin with "test". I think the word could get removed from all of them, since it doesn't seem to be adding information to say that this is a test (should be self-evidence from its presence in this file, right?)

Sorry, I don't have a place where this "principle" (if it can even be called one) is documented, nor do I know for sure that all other files in the repo don't have "test" in their description (well, that can be found with grep...). So let me know what you think about it.

I agree. The "test" prefix doesn't add anything as the description is only valid in the context of a test case :)

Would it make sense to also remove trailing 'test's? For example, "partial garden" instead of "partial garden tests".

ErikSchierboom · 2017-05-03T13:43:38Z

It appears that there now is a commit in this PR that modified the rail-fence-cipher canonical data. Is that correct?

petertseng · 2017-05-03T17:49:44Z

Please don't include my commit.

holdenout · 2017-05-04T01:56:48Z

Oops, I'm relatively inexperienced with git and did not mean to do that. I'll fix it up.

petertseng · 2017-05-04T06:48:03Z

Don't worry, we've all been there I'm sure. Feel free to yell if you need help (what I would personally do is to use an interactive rebase to remove it, but there could be various different ways to accomplish the goal so don't limit yourself to only what I say)

petertseng

Everything seems good to me. 48 hours for anyone else?

Insti · 2017-05-06T10:03:33Z

I have no objections to this being merged. 👍
(but have not reviewed it closely enough to hit the approve button.)

petertseng · 2017-05-06T18:13:57Z

"implementations": [
  {
    "track_id": "csharp",
    "url": "https://github.com/exercism/xcsharp/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "clojure",
    "url": "https://github.com/exercism/xclojure/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "ecmascript",
    "url": "https://github.com/exercism/xecmascript/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "elixir",
    "url": "https://github.com/exercism/xelixir/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "fsharp",
    "url": "https://github.com/exercism/xfsharp/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "go",
    "url": "https://github.com/exercism/xgo/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "haskell",
    "url": "https://github.com/exercism/xhaskell/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "javascript",
    "url": "https://github.com/exercism/xjavascript/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "lua",
    "url": "https://github.com/exercism/xlua/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "perl5",
    "url": "https://github.com/exercism/xperl5/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "python",
    "url": "https://github.com/exercism/xpython/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "ruby",
    "url": "https://github.com/exercism/xruby/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "scala",
    "url": "https://github.com/exercism/xscala/tree/master/exercises/kindergarten-garden"
  },
  {
    "track_id": "swift",
    "url": "https://github.com/exercism/xswift/tree/master/exercises/kindergarten-garden"
  }
]

petertseng · 2017-05-06T18:27:59Z

C#/F# have search for a kid not in the class ("Potter" for four plants).

JS/ES6/Ruby create multiple gardens with the same kids then check that the right plants are assigned to the right kids after both are created, ensuring that the multiple gardens don't improperly overwrite each other's data. (Perl has cases with the same data but doesn't create two gardens simultaneously)

Lua checks that the kids and plants are case-insensitive.

Go has a bunch of errors cases (unequal number of rows, missing newlines, odd number of columns, duplicate kid name, something other than RGCV in the diagram, kid not in the class like C#/F#)

If anyone really wants these cases in the JSON, speak up soon. I'm not considering them blockers but they are useful to note down.

holdenout · 2017-05-07T23:42:56Z

I hate to block my own PR, but I'd be willing to either add some of that in (either as tests or just in the comments). I suppose another option could be merging and then raising an issue for those little bits. Not sure what would work best for the exercism workflow. Or it could be left as is.

petertseng · 2017-05-08T03:07:33Z

Well these are good questions. Depending on what the exercise is trying to teach (fun fact! it seems from exercism/discussions#78 that we don't even know!) those tests may or may not be appropriate.

The error tests and case insensitivity test make sense and could be encoded in JSON, but don't appear to be strictly necessary.

The test with two simultaneous gardens would at least need a comment to go along with it.

I'm happy to stick with these as a first pass. Though note that I only did this exercise in one track so I am not the most knowledgeable about all the approaches!

If in the future some track creates a test generator and finds that it deletes a test that track thought was essential, at that time we may see a proposal to add those tests to x-common.

Thanks for your work!

…nts (#525) Descriptions are clearer as to the significance of the garden size, or the fact that the given student list is non-alphabetical. The tests of the middle children in the full garden are elided. The canonical data file was added in exercism/problem-specifications#770

kytrinyx · 2017-05-08T12:55:36Z

🎉 thanks @holdenout!

petertseng reviewed May 2, 2017

View reviewed changes

holdenout force-pushed the canonical-kindergarten-garden branch from 5c3af76 to fb9d6e1 Compare May 3, 2017 00:40

petertseng mentioned this pull request May 3, 2017

kindergarten-garden: Inconsistency between description and tests #659

Closed

petertseng reviewed May 3, 2017

View reviewed changes

kindergarten-garden: add canonical data

34276c0

holdenout force-pushed the canonical-kindergarten-garden branch from 67644e9 to 34276c0 Compare May 5, 2017 09:49

petertseng approved these changes May 6, 2017

View reviewed changes

behrtam mentioned this pull request May 7, 2017

Inconsistency in Kindergarten Garden exercise exercism/python#423

Closed

petertseng merged commit 03d588d into exercism:master May 8, 2017

petertseng mentioned this pull request May 8, 2017

kindergarten-garden 1.0.0.2: edit descriptions and elide middle students exercism/haskell#525

Merged

Uh oh!

kindergarten-garden: add canonical data #770

kindergarten-garden: add canonical data #770

Uh oh!

Conversation

holdenout commented May 2, 2017

Uh oh!

petertseng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petertseng May 2, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

holdenout commented May 2, 2017

Uh oh!

hilary commented May 2, 2017

Uh oh!

petertseng left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ErikSchierboom commented May 3, 2017

Uh oh!

petertseng commented May 3, 2017

Uh oh!

holdenout commented May 4, 2017

Uh oh!

petertseng commented May 4, 2017

Uh oh!

petertseng left a comment

Choose a reason for hiding this comment

Uh oh!

Insti commented May 6, 2017

Uh oh!

petertseng commented May 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petertseng commented May 6, 2017

Uh oh!

holdenout commented May 7, 2017

Uh oh!

petertseng commented May 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kytrinyx commented May 8, 2017

Uh oh!

Uh oh!

petertseng May 2, 2017 •

edited

Loading

petertseng commented May 6, 2017 •

edited

Loading

petertseng commented May 8, 2017 •

edited

Loading