Return proper error messages on failure

[zabil]

I noticed that tests were failing as follows

 1 failing
       should reject execution when another is already in progress:
      AssertionError [ERR_ASSERTION]: "Running the contributed command: 'gauge.execute' failed." == 'A Specification or Scenario is still running!'
      + expected - actual
      -Running the contributed command: 'gauge.execute' failed.
      +A Specification or Scenario is still running!

This seems to be happening because of

gauge-vscode/src/execution/gaugeExecutor.ts

Lines 52 to 54 in 59ee8cf

	if (this.executing) {
	reject(new Error('A Specification or Scenario is still running!'));
	return;

This also does not display the custom error message for example the current version of the plugin shows

After this change

 if (this.executing) {
        window.showErrorMessage('A Specification or Scenario is still running!');
         return resolve(undefined);
 }

Notice the resolve(undefined) this is added instead of reject because reject will pop the another error box in addition to showErrorMessage

Signed-off-by: Zabil Cheriya Maliackal zabilcm@gmail.com

[sriv]

This is a regression in vscode IMO. I logged in an issue here: microsoft/vscode#120898

Edit- corrected issue link

[zabil]

The ticket you linked seems to be about "Ability to disable commit message length warning."

[NivedhaSenthil]

I think the correct issue is this microsoft/vscode#120898

[zabil]

But it looks like other plugins are using windows.showErrorMessage maybe that's the right way to go about it for example.

https://github.com/eamodio/vscode-gitlens/blob/4c7c0c1a2315539f89d3102ac860d33bac30281b/src/commands/openFileOnRemote.ts#L207-L210

[sriv]

The reason we do this instead of window. showErrorMessage is testing. It's not straightforward otherwise. Maybe that's a tradeoff we can make

[zabil]

If you mean writing unit tests. The behaviour didn't change other than what to assert. For example before.

test('should reject execution when another is already in progress', async () => {
    let spec = path.join(testDataPath, 'specs', 'example.spec');
    let doc = await workspace.openTextDocument(Uri.file(spec));
    await window.showTextDocument(doc);
    commands.executeCommand(GaugeVSCodeCommands.ExecuteAllSpecs);
    try {
        await commands.executeCommand(GaugeVSCodeCommands.Execute, spec);
        throw new Error("Expected simultaneous runs to reject");
    } catch (err) {
        assert.equal(err.message, "A Specification or Scenario is still running!");
    }
}).timeout(20000);

after

test('should reject execution when another is already in progress', async () => {
    let expectedErrorMessage;
    sandbox.stub(window, 'showErrorMessage').callsFake((args) => expectedErrorMessage = args );

    let spec = path.join(testDataPath, 'specs', 'example.spec');
    let doc = await workspace.openTextDocument(Uri.file(spec));
    await window.showTextDocument(doc);
    commands.executeCommand(GaugeVSCodeCommands.ExecuteAllSpecs);
    try {
        await commands.executeCommand(GaugeVSCodeCommands.Execute, spec);
        throw new Error("Must not run new tests when others are already in progress");
    } catch {
        assert.equal(expectedErrorMessage, "A Specification or Scenario is still running!");
    }
}).timeout(20000);

There is stubbing of showErrorMessage method. But maybe it's better to make that the intention?