search

emilybache / GildedRose-Refactoring-Kata

Starting code for the GildedRose Refactoring Kata in many programming languages.
https://youtu.be/Mt4XpGxigT4
MIT License
3.75k stars 5.19k forks source link

TextTestHarness vs Unit treatment of items #269

Closed djkingCanada closed 2 years ago

djkingCanada commented 3 years ago

I did the the Java - Spock version of the Kata. The starter unit test runs against app.items, but the test harness assumes that the original inputs will be changed.

Based on the initial unit test I built my solution around access to the property, not on side effects to the original inputs. So when I ran the TextTestHarness (TextDemoApp would be a better name) things broke. Just had to change the harness to app.items and things worked as expected.

Is this an issue? An intended part of the kata? I'm willing to do a PR to change it in the Java versions if you think it's an issue.

codecop commented 3 years ago

When using Approvals (e.g. texttest) you might need to change the test driver during refactoring. That is expected. Most code samples mutate the items, which could be removed during refactoring. Changing this would need changes in all languages, as the samples are as similar as possible.

emilybache commented 3 years ago

I'm happy you had a go at the exercise and glad for your feedback! I agree with codecop - I don't think this exercise needs an update in this respect. Keeping the texttest harness up to date is part of the challenge. Legacy code is like that - it's not always clear what all clients assume is the stable exeternal interface.