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

Add test for SIGTERM functionality #878

Merged
merged 8 commits into from
Nov 8, 2016
Merged

Add test for SIGTERM functionality #878

merged 8 commits into from
Nov 8, 2016

Conversation

pakrym
Copy link
Contributor

@pakrym pakrym commented Nov 3, 2016

{
"version": "1.1.0-*",
"dependencies": {
"Microsoft.AspNetCore.Server.Kestrel": "1.1.0-*",
Copy link
Member

Choose a reason for hiding this comment

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

Circular dependency

@pakrym pakrym force-pushed the pakrym/sigterm-test branch 2 times, most recently from c04b368 to f6fb717 Compare November 3, 2016 23:52

throw new Exception($"Project root could not be found using {applicationBasePath}");
}

Copy link
Member

Choose a reason for hiding this comment

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

nit: newline

<Import Project="$(VSToolsPath)\DotNet\Microsoft.DotNet.Props" Condition="'$(VSToolsPath)' != ''" />
<PropertyGroup Label="Globals">
<ProjectGuid>fc578f4e-171c-4f82-b301-3abf6318d082</ProjectGuid>
<RootNamespace>Microsoft.AspNetCore.Hosting.FunctionalTests</RootNamespace>
Copy link
Member

Choose a reason for hiding this comment

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

Run Dougs script to clean this up

Copy link
Member

Choose a reason for hiding this comment

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

Shared version was out-of-date. I fixed that…

@pakrym pakrym force-pushed the pakrym/sigterm-test branch from f6fb717 to 02dc39e Compare November 4, 2016 16:28
@pakrym pakrym force-pushed the pakrym/sigterm-test branch from 02dc39e to 4e7de6a Compare November 4, 2016 16:30
deployer.Deploy();

// Wait for application to start
System.Threading.Thread.Sleep(1000);
Copy link
Member

Choose a reason for hiding this comment

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

can you query the HostProcess to see if it's started? Why was the delay needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Delay is to make sure hosting started IServer.


SendSIGINT(deployer.HostProcess.Id);

deployer.HostProcess.WaitForExit();
Copy link
Member

Choose a reason for hiding this comment

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

Is this going to make the test hang if something goes wrong? Any way to timeout here?


public void Start<TContext>(IHttpApplication<TContext> application)
{
_serverEvent.Set();
Copy link
Member

Choose a reason for hiding this comment

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

There's no guarantee Start will be called if there are other startup errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Then process would be killed with timeout.

"Microsoft.Extensions.Configuration": "1.1.0-*",
"Microsoft.Extensions.Configuration.CommandLine": "1.1.0-*",
"Microsoft.Extensions.Logging.Console": "1.1.0-*",
"Microsoft.Net.Http.Headers": "1.1.0-*"
Copy link
Member

Choose a reason for hiding this comment

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

not needed?

"version": "1.1.0-*",
"dependencies": {
"Microsoft.AspNetCore.Hosting": "1.1.0-*",
"Microsoft.AspNetCore.WebUtilities": "1.1.0-*",
Copy link
Member

Choose a reason for hiding this comment

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

Not needed?

}
},
"tools": {
"Microsoft.AspNetCore.Server.IISIntegration.Tools": "1.0.0-preview2-final"
Copy link
Member

Choose a reason for hiding this comment

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

remove tools and scripts

},
"publishOptions": {
"include": [
"web.config"
Copy link
Member

Choose a reason for hiding this comment

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

remove

@pakrym pakrym merged commit 1ee2797 into dev Nov 8, 2016
@muratg
Copy link

muratg commented Nov 8, 2016

@pranavkm FYI

@JunTaoLuo JunTaoLuo deleted the pakrym/sigterm-test branch May 19, 2017 17:55
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.

6 participants