Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(anagram): clean up redundant tests #251

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ofhope
Copy link

@ofhope ofhope commented Jun 12, 2023

I tried to open this a few months ago during a hiatus. Reopening again 🤞🏽 #233

Going through this exercise I notice a fair bit of redundant testing. They loose value when duplicated and make it harder to establish the expected result.

Removed multiple tests for "detects multiple anagrams"
Removed multiple tests for "anagrams must use all letters exactly once"
Removed multiple test for "case insensitive checking"
Clarified test descriptions

Copy link
Member

@jonmcalder jonmcalder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

If you'd like to update the tests for this exercise, then please align the changes to match the tests outlined in the problem specifications repo

Track tests do fall out of sync with what's specified (as has happened here), but we generally try not to deviate from those tests unless there is a track-specific or language-specific reason to do so.

@ofhope
Copy link
Author

ofhope commented Jul 26, 2023

I think I'll close this down. It looks like the specification has duplication meaning these tests would need to reflect that.

Test cases are immutable with only description, comments or scenario additions being allowed.

Test cases are immutable, which means that once a test case has been added, it never changes.

So every language track needs to implement redundant tests to align with this problem specification. It makes sense to make them immutable so the many dependant language tracks don't fall out of alignment and aren't routinely having to sync. But the OCD in me has a little eye twitch while reading this specification 😄

Duplicate test example (the candidates input are slightly different but from a test perspective I would call them identical).

{
  "uuid": "a0705568-628c-4b55-9798-82e4acde51ca",
  "description": "words other than themselves can be anagrams",
  "property": "findAnagrams",
  "input": {
    "subject": "LISTEN",
    "candidates": [
      "Listen",
      "Silent",
      "LISTEN"
    ]
  },
  "expected": [
    "Silent"
  ]
},
{
  "uuid": "33d3f67e-fbb9-49d3-a90e-0beb00861da7",
  "reimplements": "a0705568-628c-4b55-9798-82e4acde51ca",
  "description": "words other than themselves can be anagrams",
  "property": "findAnagrams",
  "input": {
    "subject": "LISTEN",
    "candidates": [
      "LISTEN",
      "Silent"
    ]
  },
  "expected": [
    "Silent"
  ]
}

@ofhope
Copy link
Author

ofhope commented Jul 26, 2023

Oh... wait I just noticed the field reimplements and see the section "Changing Tests". I'll take a closer look at this specification and see if I can align it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants