A new test for "Change"

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.

Yes, I vote"good idea"

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

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.

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.

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?

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

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.

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:

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