GemTalk / RowanClientServices

Rowan service classes for client smalltalk transport layer
MIT License
0 stars 1 forks source link

Browsing a test class under construction attempts to create instances #2

Open martinmcclure opened 3 years ago

martinmcclure commented 3 years ago

Browsing a test class attempts to create instances. If instance creation is broken for that class at that moment (due to, for instance, a broken initialize method) you get the stack below. This is fairly difficult to recover from, since you can't easily browse the class to fix it. It appears to be trying to get the list of test selectors -- it looks like there's API t(perhaps TestCase class >> allTestSelectors) to do that without creating instances.

MessageNotUnderstood >> defaultAction @9 line 7
MessageNotUnderstood (AbstractException) >> _defaultAction @4 line 4
MessageNotUnderstood (AbstractException) >> _signalWith: @5 line 25
MessageNotUnderstood (AbstractException) >> signal @2 line 47
NonLocalReturnTest (Object) >> doesNotUnderstand: @9 line 10
NonLocalReturnTest (Object) >> _doesNotUnderstand:args:envId:reason: @8 line 14
NonLocalReturnTest >> initializeRandom @14 line 12
NonLocalReturnTest >> initialize @2 line 2
NonLocalReturnTest class >> new @3 line 2
NonLocalReturnTest class (TestCase class) >> selector: @2 line 3
[] in TestCase class >> buildSuiteFromMethods: @10 line 7
[] in Collection >> inject:into: @7 line 11
OrderedCollection (Collection) >> do: @5 line 10
OrderedCollection (Collection) >> inject:into: @3 line 11
NonLocalReturnTest class (TestCase class) >> buildSuiteFromMethods: @6 line 4
NonLocalReturnTest class (TestCase class) >> buildSuiteFromSelectors @3 line 2
NonLocalReturnTest class (TestCase class) >> buildSuite @12 line 9
NonLocalReturnTest class (TestCase class) >> suite @2 line 3
NonLocalReturnTest class (GsTestCase class) >> suite @2 line 4
RowanMethodService >> initializeTestMethodsFor: @8 line 7
RowanMethodService >> forClass:organizer: @32 line 21
RowanMethodService >> initialize:organizer: @29 line 20
RowanMethodService class >> forGsNMethod:organizer: @3 line 3
RowanFrameService >> initializeProcess:level:organizer: @14 line 13
RowanFrameService class >> process:level:organizer: @3 line 4
RowanProcessService >> initialize:status: @9 line 7
RowanProcessService >> update @3 line 3
RowanProcessService (Object) >> perform:withArguments: @1 line 12
RowanProcessService >> servicePerform:withArguments: @2 line 2
[] in JadeServer >> updateFromSton: @35 line 13
OrderedCollection (Collection) >> do: @5 line 10
[] in JadeServer >> updateFromSton: @24 line 9
ExecBlock0 (ExecBlock) >> on:do: @3 line 44
[] in JadeServer >> updateFromSton: @12 line 14
ExecBlock0 (ExecBlock) >> on:do: @3 line 44
JadeServer64bit35 (JadeServer) >> updateFromSton: @2 line 23
GsNMethod class >> _gsReturnToC @1 line 11
dalehenrich commented 3 years ago

This is @ericwinger territory ,,, the culprit looks like RowanMethodService >> initializeTestMethodsFor: might need an error handler for this case (?):

NonLocalReturnTest class (GsTestCase class) >> suite @2 line 4
RowanMethodService >> initializeTestMethodsFor: @8 line 7
martinmcclure commented 3 years ago

Thanks, @dalehenrich. An error handler might be a good idea in general, but I think for this particular case creating instances is not necessary so it would be better to just not create instances.

dalehenrich commented 3 years ago

it's up to @ericwinger ...

ericwinger commented 3 years ago

I made a quick fix that sends the allTestSelectors method instead of suite which should avoid instantiating the test instances. Offered to @martinmcclure. If it works, I'll defer to him to close this issue. The fix would be in the first version of Jadeite to support V3.0.

If needed, we can pull it back to a prior version of RowanClientServices. Would like some feedback if that's required.

dalehenrich commented 3 years ago

I wouldn't be surprised if this fix would also address https://github.com/GemTalk/Jadeite/issues/888

ericwinger commented 3 years ago

Forgot to put the commit in the issue. Feel free to see if this improves the performance of https://github.com/GemTalk/Jadeite/issues/888 @dalehenrich https://github.com/GemTalk/RowanClientServices/commit/884a51da589d8f1dd254eac35602729e85a676e1

dalehenrich commented 3 years ago

I'll merge your changes into my work and test, when I get a chance ... @ericwinger will I need to have a new Jadeite version to use your latest oscarV3.0Component_eric or are the rest of your changes compatible with the currently released Jadeite?

martinmcclure commented 3 years ago

I cherry-picked Eric's commit to the martin-issue2 branch (branched from masterV2.2), and built a 3.7 Rowan extent from it. This change looks good to me, but a similar change is needed in RowanClassService>>initializeTestMethodsFor:. The test for this problem is simple -- add an initialize method in a test class that contains 3 zork. Instant walkback on accept. Assigning back to Eric. :-)

ericwinger commented 3 years ago

I wasn't able to reproduce the problem in the latest Jadeite in development for Rowan V3.0 using the reproduction case given. Nonetheless, I made a similar change as indicated in @martinmcclure 's comment: https://github.com/GemTalk/RowanClientServices/issues/2#issuecomment-929552405

I'll merge your changes into my work and test, when I get a chance ... @ericwinger will I need to have a new Jadeite version to use your latest oscarV3.0Component_eric or are the rest of your changes compatible with the currently released Jadeite?

@dalehenrich You'll want to cherry pick the two sha's below for your testing. Don't merge the entire oscarV3.0Component_eric branch.

https://github.com/GemTalk/RowanClientServices/commit/884a51da589d8f1dd254eac35602729e85a676e1 https://github.com/GemTalk/RowanClientServices/commit/1b1ece6bfe7ed76af7238cab222a19ef04b8a8af

Assigning back to @martinmcclure

martinmcclure commented 3 years ago

Created PR #3, back to Eric for approval.

martinmcclure commented 3 years ago

PR #5 has been merged to candidateV2.2. Leaving this issue open until merged to masterV2.2, since the 3.7 server is building from masterV2.2.