Test failures in positron-run-app extension

[dhruvisompura]

Describe the issue:

I'm seeing some test failures in the positron-run-app extension failing when run on the main branch:

1) PositronRunApp
       debugApplication: shell integration supported:
     AssertError: expected previewUrl to be called once and with match
  	at Object.fail (node_modules/sinon/lib/sinon/assert.js:120:25)
  	at failAssertion (node_modules/sinon/lib/sinon/assert.js:66:20)
  	at assert.<computed> [as calledOnceWithMatch] (node_modules/sinon/lib/sinon/assert.js:95:17)
  	at Context.<anonymous> (extensions/positron-run-app/out/test/api.test.js:144:22)

  2) PositronRunApp
       debugApplication: shell integration disabled, user enables and reruns:
     AssertionError [ERR_ASSERTION]: Timed out waiting for URL preview
  	at Context.<anonymous> (extensions/positron-run-app/out/test/api.test.js:166:9)

It looks like the last time the test file was touched was in: 855554f but unsure when the test failures were introduced.

Steps to reproduce the issue:

1. npm run test-extension -- -l positron-run-app

Expected or desired behavior:

Expected the debugApplication: shell integration supported and debugApplication: shell integration disabled, user enables and reruns tests to pass.

[juliasilge]

Looks to me like these tests are not run in CI, which may be an oversight:

https://github.com/posit-dev/positron/blob/main/scripts/test-integration-pr.sh

[juliasilge]

Ah, not an oversight it looks like! Note the change in scripts/test-integration-pr.sh in 855554f!

I see in Slack that these tests were flaky so we decided not to run them in CI, and @seeM said these tests are mainly useful during the local dev process for this extension.

• @seeM can you comment with anything additional on this? What do you think about these failing for @dhruvisompura?
• @dhruvisompura do you see these tests fail every time you run them? Or is it intermittent?

[seeM]

@juliasilge It's hard to say what's going on without investigating further. I did encounter some flakiness in these tests myself when developing, which is why I originally decided not to include them in CI. But I couldn't find a fix.

We have quite a few end-to-end tests covering the same behavior. So I'd be happy to ignore this until either users report it in practice or we see flakiness in the end-to-end tests.

[dhruvisompura]

• @dhruvisompura do you see these tests fail every time you run them? Or is it intermittent?

@juliasilge These tests fail every time I run them locally. Given the consistency of the failure, it could be a test setup issue on my end if this isn't reproducible by others.

[juliasilge]

To run these tests after we've moved from yarn to npm, use npm run test-extension -- -l positron-run-app.

I see the same test failures as @dhruvisompura, and this is not intermittent but instead does seem to be truly a problem with the tests as written. I know we're not running these tests in CI but it's not unreasonable to use them in the dev process, like Dhruvi was when updating lots of settings, so we should clean these up and see what has changed. I'll move this to a milestone later this spring, to be addressed during a time of polish and bug fixing.

[seeM]

Ah, it's failing every time for me as well. I'm actually also seeing this bug in Flask and Streamlit apps too!

I looked into it a bit. Sometimes onDidStartTerminalShellExecution fires before onDidRequestRunInTerminal here:

positron/extensions/positron-run-app/src/api.ts

Lines 294 to 300 in be85387

	const disposable = this._debugAdapterTrackerFactory.onDidRequestRunInTerminal(e => {
	if (e.debugSession.configuration.debugAppRequestId === debugAppRequestId) {
	// Dispose the existing terminal execution listener, if any.
	executionDisposable?.dispose();
	const { processId } = e;
	executionDisposable = vscode.window.onDidStartTerminalShellExecution(async e => {

We need to synchronize somehow so that those can fire in either order.

EDIT: I'm not sure what changed that caused this to start failing. Could be something upstream.