Fix Document.initAction to fire when opening from the IDE

[jamshark70]

Purpose and Motivation

Fixes #4549.

Previously, Document.initAction = { ... } or autorun documents took effect only when opening the document programmatically (Document.open or Document.new), but these were ignored when opening the document using IDE menu commands.

The simplest fix also means that autoRun documents will execute whenever recompiling the class library (if they are open). Per discussion under #4549, this seems to match the original autoRun behavior under Cocoa, and it's justifiable as an easy way to write "soft" startup code. However, the documentation inaccurately claimed that autoRun fires only when opening the document. So I updated the help file as well.

This PR touches two behaviors that are impossible to test automatically:

• User action in the IDE.
• Startup behavior after recompiling the library (as this would halt the entire test suite).

I've added tests for initAction and autoRun when opening a document programmatically. That's about all I think I can do.

Types of changes

• Documentation
• Bug fix

To-do list

• Code is tested
• All tests are passing
• Updated documentation
• This PR is ready for review

[dyfer]

Everything looks good to me. /*RUN*/ works when opening the document and when recompiling library, unit test passes, documentation is updated to match the fixed behavior.

[nhthn]

thanks! some comments mostly on testing.

[nhthn]

UnitTest doesn't have a built-in mechanism for marking a test as "skip", but borrowing semantics from other test frameworks, i would expect this to pass, not fail. if we put an "assert false" in here, the test suite will always fail if you are running in an environment without the IDE, which doesn't seem right to me.

i'd suggest replacing this assertion with an empty code block with a comment like // TODO: skip this test when UnitTest has skip functionality

[jamshark70]

Ok, fair enough. Should there or should there not be some record of tests that were skipped, and why, in a given run? Eventually yes, I suppose.

[nhthn]

just remembered there's an open ticket for this: #4384. i propose adding // TODO: skip to all skipped tests so we can easily grep for them later when test skipping is implemented.

[jamshark70]

Done.

[nhthn]

0.1.wait in a test is a code smell. can you justify why this is necessary?

[jamshark70]

Normal way to handle asynchronicity is with a condition.hang in the async response (initAction here). But we are testing whether the initAction fired or not. Therefore there is some reasonable expectation that the initAction may fail to fire. In that case, the Condition never unhangs and voilà, the entire test suite has ground to a halt.

This situation is impossible to test without a timeout. So, please advise what is the interface provided by the UnitTest suite for asynchronicity with timeout, and I'll very happily update (as I've been waiting years for that interface).

[jamshark70]

BTW I've started working on a Condition:hangWithTimeout so that this might not be a problem in the future.

[nhthn]

i don't work a lot with UnitTest, but i think it's UnitTest:wait? if that doesn't work, leaving in the 0.1.wait is fine as long as there is a comment justifying it for anyone looking at the code.

[jamshark70]

Oh, but there is #3895. Well, what to do. I'll email the dev list.

[nhthn]

could you try UnitTest:wait and see if it works here?

[jamshark70]

Yeah, UnitTest:wait is better. I didn't know about that one.

[jamshark70]

BTW fixed 10 days ago. AFAICS this is ready to go?

[telephon]

BTW I've started working on a Condition:hangWithTimeout so that this might not be a problem in the future.

There was a discussion about this in the past. I remember that the result was that you can just write .hang(0.5) or whatever time you want as timeout, and that it was a documentation issue.

[jamshark70]

There was a discussion about this in the past. I remember that the result was that you can just write .hang(0.5) or whatever time you want as timeout, and that it was a documentation issue.

That discussion eventually rejected that solution.

(
r = fork {
	var c = Condition.new,
	resp = OSCFunc({ c.unhang }, '/test'),
	now;
	thisThread.clock.sched(0.25, {
		"let's pretend we got an async reply in 0.25 sec".postln;
		NetAddr.localAddr.sendMsg("/test");
	});
	c.hang(2);
	now = thisThread.beats;
	loop {
		(thisThread.beats - now).debug("beats since unhanging");
		1.0.wait;
	};
};
)

let's pretend we got an async reply in 0.25 sec
beats since unhanging: 0.0
beats since unhanging: 1.0
beats since unhanging: 1.74982049
beats since unhanging: 2.0
beats since unhanging: 2.74982049

r.stop;

[mossheim]

lgtm! thanks

[mossheim]

what's the intent here with UniqueID.next?

[jamshark70]

Hm, I think my intent was to reduce the likelihood of crashing into a leftover file in the temp directory. But UniqueID probably wouldn't work for that.

I'll just take it out.

changes addressed