pharo-smalltalk icon indicating copy to clipboard operation
pharo-smalltalk copied to clipboard

Refactor ExercismGenerator to use Tonel file map

Open bencoman opened this issue 6 years ago • 6 comments

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 - ???

bencoman avatar May 29 '19 17:05 bencoman

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 [email protected] 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:

https://github.com/exercism/pharo-smalltalk/pull/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 avatar May 30 '19 00:05 macta

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 avatar May 31 '19 06:05 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 19e2cde0f8 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 avatar Jun 04 '19 18:06 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 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

bencoman avatar Jun 05 '19 16:06 bencoman

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...

macta avatar Jun 06 '19 00:06 macta

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.

bencoman avatar Jun 06 '19 00:06 bencoman

Refactoring of generator to work with v3 already done here. This PR is outdated.

Bajger avatar Feb 27 '25 08:02 Bajger