Refactor ExercismGenerator to use Tonel file map

[bencoman]

Working my way through the generation code, I've simplified it using Tonel file map.
git status after generation shows no changes except the following are deleted:

• README.md of all exercises - WIP
• robot-simulator/.meta/solution/BearingTest.class.st - ???

[macta]

Haven’t looked at your code / but I don’t understand the readme.md - that’s what you are generating for each exercise aren’t you? (It’s what configlet creates based in the template? Or is it a one off in a strange place? I recall adding one to document a directory - so look at its contents). The robot test - is part of the reference solution , it was a test to check one of the solution classes - however we don’t have to export it or the reference solution, as I don’t believe they get used (which is a shame - as you can’t show students a working example unless others have solved it)

…

Sent from my iPhone

On 29 May 2019, at 18:42, Ben Coman ***@***.***> wrote: Working my way through the generation code, I've simplified it using Tonel file map. git status after generation shows no changes except the following are deleted: README.md of all exercises - WIP robot-simulator/.meta/solution/BearingTest.class.st - ??? You can view, comment on, or merge this pull request online at: #372 Commit Summary Refactor ExercismGenerator to use Tonel file map File Changes M dev/src/ExercismDev/ExercismGenerator.class.st (74) A dev/src/ExercismTools/MCDefinition.extension.st (11) Patch Links: https://github.com/exercism/pharo-smalltalk/pull/372.patch https://github.com/exercism/pharo-smalltalk/pull/372.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[macta]

This might be why you see one difference - where it leaves out any extra tests a student might want to include in their solution (in the submit command, it uses ExercismTestCase, and we expect users to subclass TestCase if they want to include their tests (which seemed reasonable)

[bencoman]

Thanks. That is entirely reasonable. In fact its a great idea. I just wasn't aware of it.
[edit] Confirming that fixed it. But I'm not clear if you preferred it not to be included?

[bencoman]

Now generates README without using configlet
Excepting still to do...

• append source and credits. Do we wait for that? Except for custom exercises is there any data in our repo for these or we have to get it from problem-specifications?

btw, I'm thinking we should make the exercism/problem-specification repo a "dev" dependency.
Then it will be available in a standard location for the tools to look first before asking for it.
I've previously cloned it using Iceberg which worked fine.

[bencoman]

Appending "Source" and "Credits" to generated files requires reading from the problem-specifications.
To facilitate this I added new classes ExercismProblemSpecification and ExercismLabeledTest.
@macta I still didn't quite grok the recursion of #generateTestMethodsOn:calling:using:prefix:
particularly with respect to ifEmpty: [ methodNameSegment withoutPrefix: 'And' ]
so I moved the method naming to ExercismLabeledTest.
In my latest commit 19e2cde could you try ExercismProblemSpecification all inspect
and review instance variable tests showing method names that you expect?

P.S. This refactoring may assist (later) with regeneration - with a bit more liveness holding the problem-specifications in a collection and focus on individual ones.

[bencoman]

After my last commit a4f6c03 the following can be used to review the differences in method names...

newTests := Dictionary new.
pTests := ExercismProblemSpecification all flatCollect: #tests.
pTests do: [  :m | newTests at: m methodNameSegment asString put: m ].

existing := Dictionary new.
eTests := ExercismTest allSubclasses flatCollect: #methods. 
eTests do: [ :m | existing at: ((m selector findTokens: '_') last)  put: m ].

"methods to exclude"
newTests keysDo: [ :methodName | existing removeKey: methodName ifAbsent: [ ] ].
#('testHello' 
'setUp'
'EnsureDataIsImmutable' "test100"
'should:raise:containing:'
'assert:closeEnoughTo:'
) do: [ :removeMe | 
		existing removeKey: removeMe ifAbsent: [] ].
	
"Custom or queried at https://github.com/exercism/pharo-smalltalk/issues/377"
#(DieTest DieHandleTest RobotSimulatorTest LeapTest EtlTest ClockTest AllergiesTest
) do: [ :class | existing keysAndValuesDo: [ :key :value |
	value methodClass = class asClass ifTrue: [ existing removeKey: key ] ]].

existing inspect

newTests := Dictionary new.
pTests := ExercismProblemSpecification all flatCollect: #tests.
pTests do: [ :m | newTests at: m methodNameSegment asString put: m ].

existing := Dictionary new.
eTests := ExercismTest allSubclasses flatCollect: #methods.
eTests do: [ :m | existing at: ((m selector findTokens: '_') last) put: m ].

"methods to exclude"
newTests keysDo: [ :methodName | existing removeKey: methodName ifAbsent: [ ] ].
#('testHello'
'setUp'
'EnsureDataIsImmutable' "test100"
'should:raise:containing:'
'assert:closeEnoughTo:'
) do: [ :removeMe |
existing removeKey: removeMe ifAbsent: [] ].

"Custom or queried at #377"
#(DieTest DieHandleTest RobotSimulatorTest LeapTest EtlTest ClockTest AllergiesTest
) do: [ :class | existing keysAndValuesDo: [ :key :value |
value methodClass = class asClass ifTrue: [ existing removeKey: key ] ]].

existing inspect

[macta]

I still didn't quite grok the recursion of #generateTestMethodsOn:calling:using:prefix:
particularly with respect to ifEmpty: [ methodNameSegment withoutPrefix: 'And' ]
so I moved the method naming to ExercismLabeledTest.

I haven't had a chance to properly understand all that you are doing (I think you might have missed a trick to do this in some simpler stages - e.g. do something file based, expecting the relevant repos are already checked out first like the readme.md says - to get it working, and then eliminate that).

To answer your question, the recursive bit is because there are some specs that have nested cases in cases in cases. I went to find some examples however I think they might now have been changed (resister-colours was like that, and I think allergies was too). Before I only handled 2 levels deep, and then the numbering would mess up and names would get duplicated too.

I should have written some tests right then - but I was a bit past the point of no return and it was late... and I was weak...

[bencoman]

methodNameSegment withoutPrefix: 'And'

I sorted this out last night while reconciling the "would-be-generated" method names with existing names. The examples I found leftover had method names starting with "andXxxx" so I guess that was essential function of > methodNameSegment withoutPrefix: 'And'

With my new ExercismLabeledTest I also recursed the descriptions, but separate the concerns of building a list of descriptions then later generating methods names from that list. This made it easier to #inspect what was going on.