microsoft / vscode-debugadapter-node

Debug adapter protocol and implementation for VS Code.
Other
273 stars 79 forks source link

Question: Promise.all or chained? #115

Closed raix closed 7 years ago

raix commented 7 years ago

Should this run in sequence or does dc handle this for us?

test('should stop on entry', () => {
    return Promise.all([
        dc.configurationSequence(),
        dc.launch({ program: "main.js", stopOnEntry: true }),
        dc.assertStoppedLocation('entry', 1)
    ]);
});

or

test('should stop on entry', () => {
    return Promise.resolve()
        then(() => dc.configurationSequence())
        then(() => dc.launch({ program: "main.js", stopOnEntry: true }))
        then(() => dc.assertStoppedLocation('entry', 1));
    ]);
});
raix commented 7 years ago

...or

test('should stop on entry', async () => {
  await dc.configurationSequence();
  await dc.launch({ program: "main.js", stopOnEntry: true });
  await dc.assertStoppedLocation('entry', 1);
});
weinand commented 7 years ago

The use of Promise.all runs things in parallel (which is the intended behaviour). However, since configurationSequence waits for an initialized event and assertStoppedLocation waits for a stopped event, launch is the only thing that proceeds and generates the 'initialized' event. This makes the configurationSequence proceed. Later an 'stopped' event arrives which makes assertStoppedLocation succeed.

So it is not 'dc' that handles sequential execution in a generic way but the individual utility methods do this. It would have clearer if the names of the methods would contain a "wait" to better reflect this.

Your strictly sequential snippets 2 and 3 would work too, but setting up event handlers in parallel better reflects the way VS Code works.

raix commented 7 years ago

Thanks for the answer, I ended up with a mix.

  1. Setting up event listeners
  2. Calling await commands