finish not be defined in SmalltalkCI class>>travisTime:foldName:on:block: ?

[dalehenrich]

All of the GemStone builds failed for tODE last night with the same error:

ERROR 2010 , a MessageNotUnderstood occurred (error 2010), a UndefinedObject does not understand  #'-' (MessageNotUnderstood)

topaz > exec iferr 1 : stk 
==> 1 MessageNotUnderstood >> defaultAction         (envId 0) @2 line 3
2 AbstractException >> _signalWith:             (envId 0) @5 line 25
3 AbstractException >> signal                   (envId 0) @2 line 47
4 Object >> doesNotUnderstand:                  (envId 0) @9 line 10
5 Object >> _doesNotUnderstand:args:envId:reason: (envId 0) @7 line 12
6 [] in  SmalltalkCI class >> travisTime:foldName:on:block: (envId 0) @2 line 12
7 ExecBlock >> ensure:                          (envId 0) @4 line 13
8 SmalltalkCI class >> travisTime:foldName:on:block: (envId 0) @15 line 11
9 [] in  SmalltalkCI class >> travisFold:id:on:block: (envId 0) @2 line 3
10 ExecBlock >> ensure:                          (envId 0) @2 line 12
11 SmalltalkCI class >> travisFold:id:on:block:  (envId 0) @5 line 4
12 SmalltalkCI class >> travisFold:id:block:     (envId 0) @3 line 2
13 SmalltalkCI class >> stage:id:block:          (envId 0) @4 line 3
14 SmalltalkCI >> test                           (envId 0) @3 line 4
15 SmalltalkCI class >> test:named:              (envId 0) @5 line 5
16 Executed Code                                           @3 line 1
17 GsNMethod class >> _gsReturnToC               (envId 0) @1 line 1

and I can only guess that the finish temp is not defined, but after reading code, I can't figure out how it could have failed that way ...since the block does not appear to return I cannot figure out how the SmalltalkCI bug is occurring ... I've cleared the caches (should not matter) and am rerunning one of the builds on the off chance that this was a spurious error for 6 builds?

[dalehenrich]

Yep breaks when rerun .... https://travis-ci.org/dalehenrich/tode/jobs/181842761 --- the last time a build ran was 2 weeks ago and the build was green ...

looking at the changes since the last build the only possibility that I can see is that a halt is leaking from the test ... which is bug in the test --- I'll see if I can figure out what's going on ...

It is still probably worth bullet proofing the ensure block so that an error in SmalltalkCI doesn't mask these kinds of test errors ...

[dalehenrich]

Interestingly enough, this is a test case that was involved in the bug report for Issue #237 2 weeks ago (November 23)(the last green build was when I fixed the tests) and there have been 8 commits to SmalltalkCI since then, so perhaps one of those changes introduced this vulnerability?

[fniephaus]

Mh interesting, the last 8 commits seem to be unrelated actually (changes on Bash level, or for Squeak and Pharo mostly). Nothing GemStone-related IIRC. I also don't understand why a DNU is thrown in the ensure block of SmalltalkCI class>>travisTime:foldName:on:block:. Only the travis fold should be closed in there. I'll have a closer look at it tomorrow if you haven't already figured everything out by then :)

[fniephaus]

Ah ok, so you are assuming that this line before finish is set and that can only happen if aBlock value. fails. That makes sense, maybe better to ensure finish is set as part of the, yep, ensure block?

[dalehenrich]

Interestingly enough ... I'm getting the same failure when I introduce a simple test failure into the mix ... so there is a deeper problem in SmalltalkCI ---

How can a test failure cause finish to not be set?

[dalehenrich]

The last build to fail properly is this one: https://travis-ci.org/dalehenrich/tode/builds/164151779 -- 2 months ago ...

[fniephaus]

Since it's covered up, I'm running it again with the patch on dev at https://travis-ci.org/dalehenrich/tode/builds/182115985

[dalehenrich]

... and this is issue #237 .... now I looked at diffs for SmalltalkCI and didn't see any changes to this code, so why do you think this problem showed up today but didn't apply 2 weeks ago? There must have been a subtle change recently?

I was beginning to wonder about the changes made where you'd added in the errorException method ... but I didn't see anything immediately obvious

[fniephaus]

I'm not saying it didn't apply 2 weeks ago and it's very likely a problem I introduced in the last few weeks. Could you elaborate a little bit on #237? Why is this happening? Why do you think the error is caused when saving the error stack? Looks like there was a problem in TDStackFrameTests>>testExecBlockComplexTempsBaseNodeMap, and SCIGemStoneTestRunner>>stackTraceString:of: was successful?

Anyway, I need to get some rest now, will look at it tomorrow again.

[dalehenrich]

For Issue #237, the TransactionError is being signalled by the #commitTransaction in https://github.com/hpi-swa/smalltalkCI/blob/master/repository/SmalltalkCI-GemStone-Core.package/SmalltalkCIGemstone.class/class/saveImage.st ... this is only done when there are errors. The TransactionError is occurring because in GemStone one is not allowed to commit the ProcessScheduler instance ... and it is being included in the continutation that GemStone creates when saving a continuation for the error itself ... this particular test is forking processes and holding onto GsProcess instances something that is not normally done in most tests ... for tODE I am testing the debugger, so forking is required ...

For the issue #237 fix I will bullet-proof the commit and stop snapping off continuations when running a travis test --- continuations are only useful when you have interactive access to the stone ...

BTW, I will probably fixIssue #237 today, because I've built a test stone for trying to reproduce the bug locally, but you patched the problem before I was able to start running tests:)

[fniephaus]

The patch is not on master yet, so knock yourself out ;)
smalltalkCI attempts to save the image as part of SmalltalkCI>>test: only when there are any errors in order to persist them. And that's only for debugging purposes, so feel free to change the behaviour for GS if you need/want to.

[dalehenrich]

I'm still curious how finish sudenly got involved and I'm wondering if there needs to be some "higher level" error testing to ensure that errors in the build infrastructure can be properly exposed ... having errors masked as a side effect of other changes should not be surprising ....

[fniephaus]

Do you have specific ideas for such tests?

[dalehenrich]

Not really ... a human has to look at it the output to really recognize those kinds of errors ... Hmmm, you could produce a standard report that uses all of the ansi colors and styles as well as printing all of the objects used in the report and then compare the output to a good copy ... there are fields in the output that can vary from run to run like the timings ,,, but perhaps you could force a constant for the test report ...

[fniephaus]

I already have this test in place, so I guess I just need to add a check that the titles are also included in the output (except the timing).

[dalehenrich]

wait a second ... it appears that we are talking about #246 here? Why didn't that test fail? Is it a new test?

[dalehenrich]

with regards to finish I suppose it is hard to introduce unknown failures in unknown places to expose weaknesses ... hmmm ... perhaps it is worth putting an error handling in place in #test that catches the standard set of errors (Error, Halt ...) and if running in travis displays an error message and finishes up the output ... if not running in travis, just pass the exception ... this would bullet proof the test runner ...

[fniephaus]

wait a second ... it appears that we are talking about #246 here? Why didn't that test fail? Is it a new test?

Because that test only tested the header and not individual test items. I'm currently working on making it better. I'll think about your error handling suggestion afterwards :)

[dalehenrich]

haha ... cool!

[fniephaus]

Here you go. :)

With regard to your exception handling idea: that's exactly I think what the CommandLineToolSet in Squeak does. When installed as default, it quits on all errors. The default StandardToolSet would open the debugger instead. I'm not sure how error handling works in GemStone or in Pharo. But it would be good to have something similar in place.

[dalehenrich]

Well not much is really needed: [ ...] on: self exceptionSet do: [:ex | self isTravisBuild ifFalse: [ ex pass ]. .... log error and finish up ... ]

…

On 12/9/16 8:02 AM, Fabio Niephaus wrote: Here you go <https://github.com/hpi-swa/smalltalkCI/blob/b202c66e7ac837527214141ee8ad231a4993cd94/repository/SmalltalkCI-Tests.package/SCITestReporterStdoutTest.class/class/failureFixture.st>. :) With regard to your exception handling idea: that's exactly I think what the |CommandLineToolSet| in Squeak does. When installed as default, it quits on all errors. The default |StandardToolSet| would open the debugger instead. I'm not sure how error handling works in GemStone or in Pharo. But it would be good to have something similar in place. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#245 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAmFT9MVq52bGwUaLRTrVxGS9oyh7LErks5rGXuugaJpZM4LHEH->.

[fniephaus]

And you'd catch all exceptions like this in both, #load and #test, right? But I'd say it's better to check if smalltalkCI in headless mode instead of checking for Travis only.

[dalehenrich]

Not all exceptions ... but Error and Halt and platform specific "error exceptions" ... For GemStone the travisbuild is the deciding factor, because it is the only case where the developer cannot get their hands on the image at the end of the test, so it's the one case (for GemStone) where the developer cannot get their hands on the image for debugging ... headless is ambiquous for GemStone so I would think that rather than using headless as a test, you should have explicit tests implemented in the platform like: shouldQuitImage (the only use of headless at the moment) If a developer is running local "headless" jobs ... is there any reason that they can't force the isTravisBuild attribute to be true ... or perhaps ... AFAICT isTravis announces that a remote batch job is being done so take appropriate measures and should be used for the error test and even quit image? Dale

…

On 12/9/16 10:14 AM, Fabio Niephaus wrote: And you'd catch all exceptions like this in both, #load and #test, right? But I'd say it's better to check if smalltalkCI in headless mode instead of checking for Travis only. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#245 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAmFT8Zr3kL81xfAMBPjRnTFILY20MfWks5rGZqIgaJpZM4LHEH->.