exercism / problem-specifications

Shared metadata for exercism exercises.
MIT License
327 stars 543 forks source link

Transpose: add test-case #1046

Closed pheanex closed 6 years ago

pheanex commented 6 years ago

The current version of the transpose-test cases (1.0.0) seem to miss out on a case: There are two test-cases which cover the correct right-stripping of white-space ("first line longer than second line" and "many lines"), yet they do not cover the case, where you have to right-trim whitespace with multiple lines at the end (Idk if i describe it correctly). I noticed it, when I checked some implementations in the Python track which failed my following suggested test:

{
  "description": "descending line length",
  "property": "transpose",
  "input": [
    "The longest line.",
    "A longer line.",
    "A long line."
  ],
  "expected": [
    "TAA",
    "h  ",
    "ell",
    " oo",
    "lnn",
    "ogg",
    "ne ",
    "grl",
    "e i",
    "sln",
    "tie",
    " n.",
    "le",
    "i.",
    "n",
    "e",
    "."
  ]
}
rpottsoh commented 6 years ago

https://github.com/exercism/problem-specifications/blob/master/exercises/transpose/description.md

rpottsoh commented 6 years ago

Interesting. I am not seeing how the new test is different from the two cases you mentioned. Is it the fact that you are proposing three lines instead of two? Why doesn't "many lines" cover this?

I have not attempted this exercise, so I have no familiarity with it other than the documentation and test data.

pheanex commented 6 years ago

The difference is, that this test is the only one that checks if the right-side-space-trimming works on more than just the last line in "expected". "Many lines" does not cover this, since the last line of input is too long (you can see that only the last line in "expected" needs to get right-trimmed)

pheanex commented 6 years ago

I just came up with an even better test-case for this:

{
  "description": "mixed line length",
  "property": "transpose",
  "input": [
    "The longest line.",
    "A long line.",
    "A longer line.",
    "A line."
  ],
  "expected": [
    "TAAA",
    "h   ",
    "elll",
    " ooi",
    "lnnn",
    "ogge",
    "n e.",
    "glr",
    "ei ",
    "snl",
    "tei",
    " .n",
    "l e",
    "i .",
    "n",
    "e",
    "."
  ]
}

Maybe @ErikSchierboom or @rbasso want to review this?

ErikSchierboom commented 6 years ago

I think this is a very nice addition. It may even be a replacement for the existing "many lines" case, as this case handles the same problem but in a better, more interesting way.