Groxx
commented
1 year ago hmmm. no, I think I'm going to add on-started to this one. seems necessary since we already had that API exposed to some degree.
that's unfortunately more complicated than it originally seemed because children workflows have two phases of visible execution: start and complete. the func currently only supports one phase, and I'm not completely confident if I need to break it into two separately-yielding steps or if I can do it in just one...
Encountered and offending code identified by an internal user - thanks very much! Basically: children workflows in tests would return their mock value / call its func (which can block) before marking the started-future as "ready".
This both violates the obvious order of execution (start then complete), and makes it unreasonable / possibly deadlocking to use on-start-workflow listeners to control the execution of the workflow that is being started.
Since that's kinda irrational, and returning arbitrary errors from Start is not rational either (iirc only a few are possible, e.g. schedule to start timeout, as all others are completion-with-error results), this has now been changed:
This matches IRL / non-test behavior more accurately, though it does unfortunately mean some tests that worked before will now fail.
We also do not currently have a way to mock the child-started future's error, so any tests that actually were correctly checking for start-failures are now unable to be written. That's probably not too difficult to add, but it has not been done in this PR.