A new test for "Change"

Exercism Bugs & Feature Requests
1

I’d like to add a test case to Change with the following parameters:

      "input": {
        "coins": [1, 10, 11],
        "target": 20
      },

This is to catch out greedy approaches that try to use as many large coins as possible. The correct solution of course is 10 + 10, but greedily starting with 11 gives
a ten-coin solution with nine pennies.

I see that several years ago, someone checked in a change with the description “Add test for incorrect greedy algorithm”, but it only defends against a simple kind of greed, where you take the maximum possible number of coins at each step and get stuck as a result. If you moderate your greed, so that you take the maximum number that yields a solution, you will slip past this defense (ask me how I know!).

If others agree that this is a good idea, I can create a PR.

2

Yes, I vote"good idea"

3

I’m open to this. Should this replace the existing greedy test (using the reimplements field)?

4

The existing test does pretty much the same thing as the one preceding it (no pennies, get stuck if blindly greedy), so yes, I’d say “reimplements” would be a good choice.

5

Finally coming back to this, I have a PR:

I decided against “reimplements” because of the rule that the Description should stay the same–it doesn’t fit the new test.

6

P.S. I see that schema validation failed with a complaint showing four duplicate uuids. However, none of them is the uuid of my new test. How should I resolve this?

7

P.P.S. The duplicate uuids seem to be the result of copying pop-count to eluids-eggs.

8

The CI tests are all failing because exercises/eliuds-eggs/canonical-data.json and exercises/pop-count/canonical-data.json have the same UUIDs. This one is a bit unusual as it’s one of the few (only?) exercise renames.

It might be able to ignore those failures, but I’m not sure.

@iHiD / @ErikSchierboom should pop-count be removed? The duplication is going to make all PRs fail the CI tests here.

9

We can move it once all tracks have renamed their exercise. Erik will check/fix this, but he’s been off ill for the last week so we’re a bit behind :slight_smile:

10

I’ve manually fixed and merge the remaining open PRs:

irb(main):001:0> Exercise.where(slug: ‘pop-count’).count
=> 0

I’ve create a prob-specs PR to remove pop-count: Remove pop-count exercise by ErikSchierboom · Pull Request #2386 · exercism/problem-specifications · GitHub

3 Likes