Adding tests to the two bucket exercise

Exercism Building Exercism
1

Hi,

I recently solved the two bucket problem. Doing so I felt two tests were missing to cover common pitfalls.

The tests :

    #[test]
    fn test_several_pour_in_smaller() {
        let output = solve(5, 1, 2, &Bucket::One);
        let expected = Some(BucketStats {
            moves: 6,
            goal_bucket: Bucket::One,
            other_bucket: 1,
        });
        assert_eq!(output, expected);
    }

    #[test]
    fn test_several_pour_in_bigger() {
        let output = solve(3, 15, 9, &Bucket::One);
        let expected = Some(BucketStats {
            moves: 6,
            goal_bucket: Bucket::Two,
            other_bucket: 0,
        });
        assert_eq!(output, expected);
    }

Those tests are usfull when we try to solve the exercise using diophantine equations (ax-by=c) with (a,b) capacities, c goal and (x,y) number of filling/emptying
using the euler extended algorythm doesnā€™t guarantee to give positive solutions for x and y. We must adapt it a bit for the exercise. Those tests covers this and one will catch x<0, the other y<0.

In the c++ community solution I saw several solutions that would break at least one of those tests. I donā€™t know if itā€™s possible to run those on all exercise to verify itā€™s useful.

Also Iā€™d really like to contribute to exercism and it could be a nice first contribution for me.

I understood that we should first modify problem-specifications/exercises/two-bucket/canonical-data.json at main Ā· exercism/problem-specifications Ā· GitHub before adapting the different languages.

I read a lot of docs on how to contribute but there is so much stuff Iā€™m a bit lost, I understood it should be discussed here but Iā€™m not sure how to continue.

I solved the exercise and wrote those tests in C++ and Rust. I could contribute also on other languages.

Any feedback/advice/guidance would be appreciated.

1 Like
2

Once the new tests have been added to the canonical data, itā€™s up to the individual track maintainers to incorporate them. The recently created ā€œRun Configlet Syncā€ GitHub workflow will help this effort by creating issues that new tests are available.

BTW, I have confirmed those tests in my bash solution.

1 Like
3
  1. Create a thread explaining what you think ought to change and why.
  2. Get buy in from three maintainers.
  3. Create a PR on the problem specs repo.
  4. Have the PR merged.
  5. Maintainers sync data from the problem specs to their tracks.
4

Could you explain in simple terms why those tests would be needed without requiring one is familiar with any specific method/algorithm?

5

Thanks for the roadmap,

Could you explain in simple terms why those tests would be needed without requiring one is familiar with any specific method/algorithm?

Yes, functionally test_several_pour_in_smaller is specific because the solution imply to pour the first bucket several times in the second before filling it again. There is no other tests like that.

The second one is the opposite, We have to pour several times from the first to the second one before emptying the second one.
After checking, there is already one test a bit like that:

"With the same buckets but a different goal, then it is possible"

"bucketOne": 6,
"bucketTwo": 15,

So it may be less functional needed. However it helped me to catch some bugs and itā€™s not really the purpose of With the same buckets but a different goal, then it is possible to verify this.

6

To be very clear, did you have code that passed all the other tests, including "With the same buckets but a different goal, then it is possible", but would fail on those newly proposed tests? Would one of those newly proposed tests catch all the bugs in your code or are both needed to catch different bugs?

7

A functional difference between test_several_pour_in_bigger and "With the same buckets but a different goal, then it is possible" is that we never need to empty the second bucket. It translates to a y=0 with the Diophantine algorithm which is a particular case. I think thatā€™s why I added it in the first place.

8

To be very clear, did you have code that passed all the other testsā€¦

Yes, they catch different bugs. Do I have to provide a code that fail? For the first one I can, for the second one Iā€™ll need a moment to find the failing code that push me to add it. Itā€™s been a week and I donā€™t remember exactly.

9

The test_several_pour_in_smaller would catch a bug in this community solution for example : efwhofmann's solution for Two Bucket in C++ on Exercism

10

I donā€™t need to see the code :slight_smile: Iā€™ll take your word for it. I just want to ascertain that every test here expands coverage, and for code that isnā€™t specifically written just to fail that one test case.

1 Like
11

Yes, in both cases I first did a solution TDD style. Once I had a solution that made all tests green I began to think about corner cases that would make my solution fail and thatā€™s how I came to those 2 tests

1 Like
12

Thank you for the explainations.

Those additional tests seem reasonable to me. Thatā€™s a :+1: from one maintainer.

1 Like
13

:+1: from me too.

1 Like
14

Get buy in from three maintainers.

So only one more :slight_smile:

@siebenschlaefer, I saw you solved this exercise using diophantine equation too in c++ so I guess it should be interesting for you ;)

15

Iā€™m :+1: as well. :slightly_smiling_face:

1 Like
16

I didnā€™t know about diophantine equations and now I want to revisit my solutions for this exercise =)

1 Like
17

Feel free to give maintainers a bit more time to check in on this. Or feel free to proceed to a PR; youā€™ve got a go-ahead from three maintainers. The PR may need a bit of time for maintainers to check in on, too.

18

Iā€™m too exited doing my first PR to the exercism repo to wait more XD

But sure, feel free to refrain me and take your time for reviewing.

It has been closed by the bot but it seems to be part of the process.

2 Likes
19

I reopened it. The bot doesnā€™t know what has or has not been discussed on the forum.