[Sattelite] Insufficient tests

[tamireinhorn]

This exercise presents an interesting challenge, specially after solving Binary Search Trees. However, for some reason, its trees do not use numerical values, and even the letter values do not seem to follow the usual alphabetical logic as far as I can see. Coupling that with the fact that of the 7 tests, only 2 are not exception verifications, I think the exercise could have more tests, derived from the trees in the Binary Search Trees exercise. I've even written them in my repo for solutions.

[github-actions]

🤖   🤖

Hi! 👋🏽 👋 Welcome to the Exercism Python Repo!

Thank you for opening an issue! 🐍  🌈 ✨

•   If you are requesting support, we will be along shortly to help. (generally within 72 hours, often more quickly).
•   Found a problem with tests, exercises or something else??  🎉
    ◦ We'll take a look as soon as we can & identify what work is needed to fix it. (generally within 72 hours).

​          ◦ If you'd also like to make a PR to fix the issue, please have a quick look at the Pull Requests doc.
             We  💙  PRs that follow our Exercism & Track contributing guidelines!

•   Here because of an obvious (and small set of) spelling, grammar, or punctuation issues with one exercise,
    concept, or Python document?? 🌟 Please feel free to submit a PR, linking to this issue. 🎉

‼️  Please Do Not ‼️ ​        ❗ Run checks on the whole repo & submit a bunch of PRs.               This creates longer review cycles & exhausts reviewers energy & time.               It may also conflict with ongoing changes from other contributors. ​        ❗ Insert only blank lines, make a closing bracket drop to the next line, change a word               to a synonym without obvious reason, or add trailing space that's not an EOL for the very end of text files.         ❗ Introduce arbitrary changes "just to change things" .         ...These sorts of things are not considered helpful, and will likely be closed by reviewers.

• For anything complicated or ambiguous, let's discuss things -- we will likely welcome a PR from you.
• Here to suggest a feature or new exercise?? Hooray! Please keep in mind Chesterton's Fence.
  Thoughtful suggestions will likely result faster & more enthusiastic responses from maintainers.

💛  💙  While you are here... If you decide to help out with other open issues, you have our gratitude 🙌 🙌🏽.
Anything tagged with [help wanted] and without [Claimed] is up for grabs.
Comment on the issue and we will reserve it for you. 🌈 ✨

[BethanyG]

Hi @tamireinhorn 👋🏽

Thank you for filing this issue. I agree: the tests look thin for statellite, and it could benefit from more tests being added.

We pull our data for satellite from the problem-specifications repo, and auto-generate the tests you see here on the track.

So I would recommend that you PR your test cases to that repo, so that all tracks can discuss and benefit from them. Once the change is made in problems-specifications, we can pull it into the Python track and re-generate the test file for the exercise.

[tamireinhorn]

If I understand correctly, I'd just have to update the following file ( https://github.com/exercism/problem-specifications/blob/main/exercises/satellite/canonical-data.json) right? In that case, what do I put as the uid for them?

…

On Tue, Jun 14, 2022 at 7:39 PM BethanyG ***@***.***> wrote: Hi @tamireinhorn <https://github.com/tamireinhorn> 👋🏽 Thank you for filing this issue. I agree: the tests look thin for statellite, and it could benefit from more tests being added. We pull our data for satellite from the problem-specifications <https://github.com/exercism/problem-specifications/blob/main/exercises/satellite/canonical-data.json> repo, and auto-generate the tests you see here on the track. So I would recommend that you PR your test cases to that repo, so that all tracks can discuss and benefit from them. Once the change is made in problems-specifications, we can pull it into the Python track and re-generate the test file for the exercise. — Reply to this email directly, view it on GitHub <#3114 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI5HUY5EK2VBHN234YLB5IDVPCYUDANCNFSM5YXSYXEA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[BethanyG]

Hi @tamireinhorn,

That is correct -- you will want to update the canonical-data.json for the exercise. 😄

You can find the specifications for the file under the README -- Test Data. Any valid UUID(v4) will work. I often use this web-based UUID Generator to generate the ids, but you can use any method, as long as they're valid UUID(v4).

I ... think that's all that is needed, but make sure to read through the linked README, as I may have forgotten something.

[tamireinhorn]

Here's the PR

[tamireinhorn]

Still not enough reviews there, sadly

[BethanyG]

My apologies @tamireinhorn - I've been really swamped. As soon as I get time to go back through the exercise, I will go ahead and add my review for your proposed changes. Please bear with me. 😄

[tamireinhorn]

No worries, thanks for the update!

…

On Wed, Jul 27, 2022 at 7:13 PM BethanyG ***@***.***> wrote: My apologies @tamireinhorn <https://github.com/tamireinhorn> - I've been really swamped. As soon as I get time to go back through the exercise, I will go ahead and add my review for your proposed changes. Please bear with me. 😄 — Reply to this email directly, view it on GitHub <#3114 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AI5HUY5QRUUI4JHP62YB72DVWFN27ANCNFSM5YXSYXEA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[BethanyG]

As a note the problems-specifications PR is still pending with a note from Glenn from 6 days ago about some additional test case scenarios and proposals.

[BethanyG]

Closing this as there is an issue open in problem specifications.