Skip to content
This repository has been archived by the owner on Dec 6, 2022. It is now read-only.

Fix several bugs to make the tests run deterministically #852

Merged
merged 4 commits into from
Jun 3, 2019

Conversation

digeff
Copy link
Contributor

@digeff digeff commented May 31, 2019

MSFT_TEST_TIMEOUT_MULTIPLIER: Modify the multiplier to have the tests timeout faster in your Dev Machine (Useful when the tests are failing).
test/int/adapter.test.ts: Updated the tests to generate a DA log
test/int/fixtures/launchProject.ts: Updated to create web server before the debug-adapter, and to return the web-server url
test/int/fixtures/launchWebServer.ts: Updated to wait for the async methods of the server, to launch on a random launchProject, and return the url as a property and inside the launch configuration
test/int/framework/frameworkTestSupport.ts: The port is randomly choosen by the web-server, so we removed the url from the test spec
test/int/puppeteer/launchPuppeteer.ts: Use a random port for the debug adapter. Disconnect the debug-adapter, and close puppeteer and chrome on cleanUp
test/int/puppeteer/puppeteerSuite.ts: Add the launch project to the context so the tests can access the web-server url
test/int/stackTrace.test.ts & test/int/wizards/breakpoints/implementation/stackTraceObjectAssertions.ts: Because the url is not fixed any more, it's more difficult to feed it to the StackTraceValidator, so pass a full url instead of a relative one instead
test/int/testSetup.ts: Kill chrome.exe before the tests, it's more reliable

.vscode/launch.json Outdated Show resolved Hide resolved
test/int/adapter.test.ts Outdated Show resolved Hide resolved
}

public get url(): URI {
const address = this._server.address();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the server object is just passed around so you can get the address, how about just returning the address? Do you want tests messing with the server in other ways?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to return one of the servers to be able to call .close() on the cleanUp.
At the moment the _server variable is private, so the tests can't mess with it (yet)...

.vsts/job.yml Outdated Show resolved Hide resolved
@digeff digeff force-pushed the async_server branch 5 times, most recently from 751caf2 to e7523d3 Compare June 2, 2019 04:29
@digeff
Copy link
Contributor Author

digeff commented Jun 3, 2019

Note: There are still some race conditions that haven't been solved by this PR. I think the current builds are passing with a 50% chance or so. Given that this PR seems to be making things better, I'm inclined to merge it as-is.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants