search

exercism / pharo-smalltalk

Exercism exercises in Pharo.
https://exercism.org/tracks/pharo-smalltalk
MIT License
34 stars 28 forks source link

Query - reconciling existing tests with problem-specifications #377

Open bencoman opened 5 years ago

bencoman commented 5 years ago

@macta, To validate my refactoring of generation, I'm comparing existing tests to what "would" be generated. There are some anomalies that may just need some background info. To try to keep things organised, I'm going to put a query per post. Could you each of my posts if you have any comment.

bencoman commented 5 years ago

[ben:] GradeSchoolTest>>#test100_EnsureDataIsImmutable doesn't have a corresponding problem-specification and its jump to number 100 indicates it was a manual entry?

[macta:] Yes - its in the category “tests-extra” ( I wondered if regenerating might be able preserve any extra tests we might feel are suitable?)

bencoman commented 5 years ago

CLEARED. [ben:] RobotSimulatorTest>>test15 (to test18) WhereRTurnRightLTurnLeftAndAAdvance.... WhereREqualsTurnRightLEqualsTurnLeftAndAEqualsAdvance.... is different when dealing with "equals" sign

[macta:] I think you’ve fully understood the problem now ;). - the definitions are a bit all over the place. I’ve managed to get a few upstream corrections approved but there are quite a few of them.

[ben:] I've updated problem-specifications https://github.com/exercism/problem-specifications/pull/1525 The difference due to version change will be okay.

bencoman commented 5 years ago

CLEARED [ben:] LeapTest>>#test01_YearNotDivisibleBy4CommonYear and all the rest of the methods are missing the "In" from "InCommonYear". I'd guess this is a legit difference due to an updated description... https://github.com/exercism/problem-specifications/commit/18875ece112eb46c7227e7e1745432ed16030ddd

[macta:] Yes, I proposed an upstream change

bencoman commented 5 years ago

CLEARED [ben:] EtlTest>>#test01_TransformsASingleLetter in my name mapping becomes... test....TransformsTheASetOfScrabbleDataPreviouslyIndexedByTheTileScoreToASetOfDataIndexedByTheTileLetterASingleLetter

Was this super big method name by chance manually slimmed down? Anyway, I'm fixing the problem-specification to match the current method name... https://github.com/exercism/problem-specifications/pull/1526

[macta:] I don’t recall this one - It does like it was manually adjusted, but I don’t recall doing that (and have tried not to and either made a pr upstream or improved the generator).

[ben:] Problem-specifications descriptions have been slimmed down So needs to be regenerated.

bencoman commented 5 years ago

SKIP - Leave for existing issue https://github.com/exercism/pharo-smalltalk/issues/246

[ben:] ClockTest>>#test27_AddMinutesAddMoreThanOneDay1500Min25Hrs compares to... _AddMinutesAddMoreThanOneDay1500MinEquals25Hrs

So again a difference due to the conversion of ""=" to "equals". Was this a late addition? Or at least after ClockTest was generated?

Seems like "equals" conversion was introduced 2 months ago... https://github.com/exercism/pharo-smalltalk/blame/91748c1d532a6c3f147c98938c27fc4848e75c4a/dev/src/ExercismDev/String.extension.st

[macta:] Clock was an awkward one - hence #246 <#246> and some discussions on exercism problem spec. I did transformations using the deprecation tool, but it was laborious and unsatisfying. I think there should be a meta hint that this is an equality test and then the generator could do the right thing (and we would have some equality generation policy).

bencoman commented 5 years ago

CLEARED [ben:] Allergies is strange. In image are only 20 test methods but the problem specification has 49 tests. I can match about half of the test methods to problem-specifications, but the rest leaves me stumped.

[macta:] Allergies, I made a pr upstream as it was a mess - this caused lots of discussion and change - the generated version is 1.2.1 its now at 2, so this is where regeneration would be helpful (we detect a higher version and then generate on top - hopefully in a diffable way?)

[ben:] Many tests added by macta's upstream change

bencoman commented 5 years ago

CLEARED [ben:] AnagramTest>>#test10_DoesNotDetectAnAnagramIfTheOriginalWordIsRepeated has an "An" versus "DoesNotDetectAAnagramIfTheOriginalWordIsRepeated generated from problem-specification... https://github.com/exercism/problem-specifications/blob/master/exercises/anagram/canonical-data.json#L94 So was that manually corrected?

[macta:] I think this one was user submitted by Ray, so he might have corrected it without doing a pr upstream.

[ben:] Corrected upstream

macta commented 5 years ago

I think you’ve understood all of these (with the added info about the #generator method)

On 5 Jun 2019, at 12:56, Ben Coman notifications@github.com wrote:

@macta https://github.com/macta, To validate my refactoring of generation, I'm comparing existing tests to what "would" be generated. There are some anomalies that may just need some background info. To try to keep things organised, I'm going to put a query per post. Could you each of my posts if you have any comment.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/exercism/pharo-smalltalk/issues/377?email_source=notifications&email_token=ABKL3VZHHETT3X2SWUOKVI3PY6SXVA5CNFSM4HTWIO3KYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GXYAITQ, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKL3V42XPRLZD4BUVG2J6LPY6SXVANCNFSM4HTWIO3A.