Update Pangram test case from cannonical data

[ricemery]

Update Pangram test case from cannonical data. The test suite is out of date.

• Pangram: Add the non-ascii test cases problem-specifications#741

[ricemery]

Oops this is a dupe of #331.

[lpalma]

I just spotted an approved iteration for this exercise with a bug that is not caught in the current test cases:

object Pangrams {
  def isPangram(s: String): Boolean =
    s.filter(_.isLetter)
      .distinct
      .length == 26
}

The bug is with sentences containing upper and lower case of the same character,
which is caught in this test case

[ricemery]

Have you thought of another test case that could be added that would indicate the code is incorrect? Thanks

…

On Thu, May 11, 2017 at 11:05 AM Luciano Palma ***@***.***> wrote: I just spotted an *approved* iteration for this exercise with a bug that is not caught in the current test cases: object Pangrams { def isPangram(s: String): Boolean = s.filter(_.isLetter) .distinct .length == 26 } The bug is with sentences containing upper and lower case of the same character, which is caught in this test case <https://github.com/exercism/x-common/blob/master/exercises/pangram/canonical-data.json#L63-L66> — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#349 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABeHG0073LBRXviVlVmkJKuCN5fnyoESks5r403pgaJpZM4Mr70v> .

[lpalma]

Have you thought of another test case that could be added that would indicate the code is incorrect?

not really. Since the test case mentioned already catches this particular bug, I think it is enough to add it to the track. I don't see much value in having multiple tests for the same scenario.

In any case, should we update the exercise, or wait for #331?

My initial comment was more like calling our attention to reassure the importance of keeping tests up-to-date.

[ricemery]

I am confused.. What do you mean by "approved" iteration for the exercise? Looking at the canonical test data and comparing to the test suite - the "upper and lower case versions of the same character should not be counted separately" test case is included. I beleive that the test suite was generated from the most recent canonical data. The example solution at https://github.com/exercism/xscala/blob/master/exercises/pangram/example.scala passes the test suite. Thanks

…

On Thu, May 11, 2017 at 12:00 PM Luciano Palma ***@***.***> wrote: Have you thought of another test case that could be added that would indicate the code is incorrect? not really. Since the test case mentioned already catches this particular bug, I think it is enough to add it to the track. I don't see much value in having multiple tests for the same scenario. In any case, should we update the exercise, or wait for #331 <#331>? My initial comment was more like calling our attention to reassure the importance of keeping tests up-to-date. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#349 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABeHG2XhuA0GSLve4OBS4yLT4PENHfRIks5r41q0gaJpZM4Mr70v> .

[lpalma]

I am confused..
What do you mean by "approved" iteration for the exercise?

Ah, sorry about that. Maybe Approved is not the right word.

Firstly, the code I mentioned in my first comment is a student's solution.

The student submitted the code and another student gave a thumbs up (that's what I meant by approved). I spotted the bug because I was checking other students solutions and was surprised to see that this student's code passed the tests.
So, I commented in the student's iteration, informing the bug and the lack of test coverage, then I came
here to open an issue, but saw this related issue already opened.

The fact that the student's solution passed the tests and was even complimented (pressed the Looks great! button) by other student raised an alert on my head, about the importance of keeping tests up-to-date.

[ricemery]

Oh, ok. The "students" solution does pass the unit tests including the last test - "upper and lower case versions of the same character should not be counted separately". I think that test case has a bit of a problem. If the uppercase letters are treated separately from lowercase letters then the number of unique chars ends up being 27. Which returns false from the method. If the uppercase letters are treated properly and combined with the lowercase letters than 24 is the number of unique chars. Which ends up returning false from the method. So.. The test case works regardless of how uppercase letters are handled. Possibly a better testcase would be to have a test case that expects true. And, include uppercase chars that have duplicate lowercase chars in the string. test("upper and lower case versions of the same character should not be counted separately") { Pangrams.isPangram("the 1 quick brown fox jumps over the 2 lazy DOGS") should be (true) }

…

On Thu, May 11, 2017 at 1:55 PM Luciano Palma ***@***.***> wrote: I am confused.. What do you mean by "approved" iteration for the exercise? Ah, sorry about that. Maybe *Approved* is not the right word. Firstly, the code I mentioned in my first comment is a student's solution. The student submitted the code and another student gave a *thumbs up* (that's what I meant by *approved*). I spotted the bug because I was checking other students solutions and was surprised to see that this student's code passed the tests. So, I commented in the student's iteration, informing the bug and the lack of test coverage, then I came here to open an issue, but saw this related issue already opened. The fact that the student's solution passed the tests and was even *complimented* (pressed the Looks great! button) by other student raised an alert on my head, about the importance of keeping tests up-to-date. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#349 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABeHG92Gr-NTCW_OFzuC1r4gAz7xd8wAks5r43XBgaJpZM4Mr70v> .

[abo64]

@lpalma You are right, I just oversaw the problem with case in that solution. So I retracted my "approval" thumbs up.

The thing to do here IMO is to

• have a list of all exercises either in Sync Up Existing Test Suites with x-common #331 or a new issue.
• then go through add test suite generators to all of them and update their test suite.
• install a mechanism that checks whether the test suite version is out of date.

All in all a major endeavour but probably inevitable?

[ricemery]

Closing. The original issue was addressed.