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

Adds hosting model as an extra deployment parameter #1261

Merged
merged 7 commits into from
Nov 10, 2017
Merged

Adds hosting model as an extra deployment parameter #1261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d108d69
adds hosting model to deployment parameters
jkotalik Nov 9, 2017
2a7433d
fixcomment
jkotalik Nov 9, 2017
7a075d6
hostingModel is case sensitive
jkotalik Nov 9, 2017
8f38fd0
logging
jkotalik Nov 9, 2017
44306e3
Feedback
jkotalik Nov 9, 2017
f0996f1
ordering on enum
jkotalik Nov 9, 2017
009b200
ordering change
jkotalik Nov 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/DeploymentParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ public DeploymentParameters(

public string PublishedApplicationRootPath { get; set; }

public HostingModel HostingModel { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Set a default value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Other values don't have default values (including enums). See: https://github.com/aspnet/Hosting/pull/1261/files#diff-8478a3aebbec4604e8f2cf79bb152195R105

Copy link
Contributor

@JunTaoLuo JunTaoLuo Nov 9, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

By default, it would take the first value as the default (or whatever is the enum equivalent of 0). Do we want to default all tests to InProcess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably should be out of process.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm okay with changing the order of the enums but personally I prefer the more explicit approach where we just set the default value. Takes out all ambiguity.


/// <summary>
/// Environment variables to be set before starting the host.
/// Not applicable for IIS Scenarios.
Expand Down
11 changes: 11 additions & 0 deletions src/Microsoft.AspNetCore.Server.IntegrationTesting/Common/HostingModel.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.AspNetCore.Server.IntegrationTesting
{
public enum HostingModel
{
OutOfProcess,
InProcess
}
}
7 changes: 7 additions & 0 deletions src/Microsoft.AspNetCore.Server.IntegrationTesting/Deployers/IISExpressDeployer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,13 @@ public override async Task<DeploymentResult> DeployAsync()

DeploymentParameters.ServerConfigLocation = Path.GetTempFileName();

if (serverConfig.Contains("[HostingModel]"))
{
var hostingModel = DeploymentParameters.HostingModel.ToString();
serverConfig.Replace("[HostingModel]", hostingModel);
Logger.LogDebug("Writing HostingModel '{hostingModel}' to config", hostingModel);
}

Logger.LogDebug("Saving Config to {configPath}", DeploymentParameters.ServerConfigLocation);

if (Logger.IsEnabled(LogLevel.Trace))
Expand Down