Browsing a test class under construction attempts to create instances

[martinmcclure]

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]

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]

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]

it's up to @ericwinger ...

[ericwinger]

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]

I wouldn't be surprised if this fix would also address GemTalk/Jadeite#888

[ericwinger]

Forgot to put the commit in the issue. Feel free to see if this improves the performance of GemTalk/Jadeite#888 @dalehenrich
884a51d

[dalehenrich]

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]

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]

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: #2 (comment)

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.

884a51d
1b1ece6

Assigning back to @martinmcclure

[martinmcclure]

Created PR #3, back to Eric for approval.

[martinmcclure]

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.