Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query - reconciling existing tests with problem-specifications #377

Open
bencoman opened this issue Jun 5, 2019 · 8 comments
Open

Query - reconciling existing tests with problem-specifications #377

bencoman opened this issue Jun 5, 2019 · 8 comments

Comments

@bencoman
Copy link
Contributor

bencoman commented Jun 5, 2019

@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
Copy link
Contributor Author

bencoman commented Jun 5, 2019

[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
Copy link
Contributor Author

bencoman commented Jun 5, 2019

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 exercism/problem-specifications#1525
The difference due to version change will be okay.

@bencoman
Copy link
Contributor Author

bencoman commented Jun 5, 2019

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... exercism/problem-specifications@18875ec

[macta:] Yes, I proposed an upstream change

@bencoman
Copy link
Contributor Author

bencoman commented Jun 5, 2019

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... exercism/problem-specifications#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
Copy link
Contributor Author

bencoman commented Jun 5, 2019

SKIP - Leave for existing issue #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
Copy link
Contributor Author

bencoman commented Jun 5, 2019

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
Copy link
Contributor Author

bencoman commented Jun 5, 2019

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

bencoman added a commit to bencoman/pharo-smalltalk that referenced this issue Jun 5, 2019
@macta
Copy link
Contributor

macta commented Jun 5, 2019 via email

@exercism exercism deleted a comment from macta Jun 14, 2019
@exercism exercism deleted a comment from macta Jun 14, 2019
@exercism exercism deleted a comment from macta Jun 14, 2019
@exercism exercism deleted a comment from macta Jun 14, 2019
@exercism exercism deleted a comment from macta Jun 14, 2019
@exercism exercism deleted a comment from macta Jun 14, 2019
@exercism exercism deleted a comment from macta Jun 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants