Skip to content
This repository has been archived by the owner on Apr 20, 2023. It is now read-only.

Commit

Permalink
Adding MigrateWebSdkRule to the DefaultMigrationRuleSet (#4963)
Browse files Browse the repository at this point in the history 
* Adding MigrateWebSdkRule to the DefaultMigrationRuleSet and adding a E2E test to cover it.

* Do not migrate compile and EmbeddedResources for web application, because those are included in the Web Sdk already.

* Addressing code review comments
  • Loading branch information
Livar authored and Piotr Puszkiewicz committed Dec 8, 2016
1 parent 5b0801c commit 8cec61c
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 32 deletions.
3 changes: 2 additions & 1 deletion src/Microsoft.DotNet.ProjectJsonMigration/DefaultMigrationRuleSet.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ internal class DefaultMigrationRuleSet : IMigrationRule
new MigrateAssemblyInfoRule(),
new RemoveDefaultsFromProjectRule(),
new CleanOutputProjectRule(),
new SaveOutputProjectRule()
new SaveOutputProjectRule(),
new MigrateWebSdkRule()
};

public void Apply(MigrationSettings migrationSettings, MigrationRuleInputs migrationRuleInputs)
Expand Down
69 changes: 63 additions & 6 deletions src/Microsoft.DotNet.ProjectJsonMigration/Rules/MigrateBuildOptionsRule.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,9 @@ public void Apply(MigrationSettings migrationSettings, MigrationRuleInputs migra

var compilerOptions = projectContext.ProjectFile.GetCompilerOptions(null, null);

var project = migrationRuleInputs.DefaultProjectContext.ProjectFile;
var projectType = project.GetProjectType();

// If we're in a configuration, we need to be careful not to overwrite values from BuildOptions
// without a configuration
if (_configurationBuildOptions == null)
Expand All @@ -207,7 +210,9 @@ public void Apply(MigrationSettings migrationSettings, MigrationRuleInputs migra
propertyGroup,
itemGroup,
_transformApplicator,
migrationSettings.ProjectDirectory);
migrationSettings.ProjectDirectory,
projectType,
csproj);
}
else
{
Expand All @@ -217,7 +222,9 @@ public void Apply(MigrationSettings migrationSettings, MigrationRuleInputs migra
propertyGroup,
itemGroup,
_transformApplicator,
migrationSettings.ProjectDirectory);
migrationSettings.ProjectDirectory,
projectType,
csproj);
}
}

Expand All @@ -227,7 +234,9 @@ private void PerformConfigurationPropertyAndItemMappings(
ProjectPropertyGroupElement propertyGroup,
ProjectItemGroupElement itemGroup,
ITransformApplicator transformApplicator,
string projectDirectory)
string projectDirectory,
ProjectType projectType,
ProjectRootElement csproj)
{
foreach (var transform in _propertyTransforms)
{
Expand All @@ -243,7 +252,13 @@ private void PerformConfigurationPropertyAndItemMappings(
foreach (var includeContextTransformExecute in _includeContextTransformExecutes)
{
var nonConfigurationOutput = includeContextTransformExecute(compilerOptions, projectDirectory);
var configurationOutput = includeContextTransformExecute(configurationCompilerOptions, projectDirectory);
var configurationOutput =
includeContextTransformExecute(configurationCompilerOptions, projectDirectory);

configurationOutput = RemoveDefaultCompileAndEmbeddedResourceForWebProjects(
configurationOutput,
projectType,
csproj);

transformApplicator.Execute(configurationOutput, itemGroup, mergeExisting: true);
}
Expand Down Expand Up @@ -283,7 +298,9 @@ private void PerformPropertyAndItemMappings(
ProjectPropertyGroupElement propertyGroup,
ProjectItemGroupElement itemGroup,
ITransformApplicator transformApplicator,
string projectDirectory)
string projectDirectory,
ProjectType projectType,
ProjectRootElement csproj)
{
foreach (var transform in _propertyTransforms)
{
Expand All @@ -292,13 +309,40 @@ private void PerformPropertyAndItemMappings(

foreach (var includeContextTransformExecute in _includeContextTransformExecutes)
{
var transform = includeContextTransformExecute(compilerOptions, projectDirectory);

transform = RemoveDefaultCompileAndEmbeddedResourceForWebProjects(
transform,
projectType,
csproj);

transformApplicator.Execute(
includeContextTransformExecute(compilerOptions, projectDirectory),
transform,
itemGroup,
mergeExisting: true);
}
}

private IEnumerable<ProjectItemElement> RemoveDefaultCompileAndEmbeddedResourceForWebProjects(
IEnumerable<ProjectItemElement> transform,
ProjectType projectType,
ProjectRootElement csproj)
{
if(projectType == ProjectType.Web)
{
var itemsToRemove = transform.Where(p =>
p != null &&
p.Include.Contains("**\\*") &&
(p.ItemType == "Compile" || p.ItemType == "EmbeddedResource"));

CleanExistingItems(csproj, new [] {"Compile", "EmbeddedResource"});

transform = transform.Where(p => !itemsToRemove.Contains(p));
}

return transform;
}

private void CleanExistingProperties(ProjectRootElement csproj)
{
var existingPropertiesToRemove = new [] {"OutputType", "TargetExt"};
Expand All @@ -314,6 +358,19 @@ private void CleanExistingProperties(ProjectRootElement csproj)
}
}

private void CleanExistingItems(ProjectRootElement csproj, IEnumerable<string> itemsToRemove)
{
foreach (var itemName in itemsToRemove)
{
var items = csproj.Items.Where(i => i.ItemType == itemName);

foreach (var item in items)
{
item.Parent.RemoveChild(item);
}
}
}

private IncludeContext GetCompileIncludeContext(CommonCompilerOptions compilerOptions, string projectDirectory)
{
// Defaults from src/Microsoft.DotNet.ProjectModel/ProjectReader.cs #L596
Expand Down
21 changes: 21 additions & 0 deletions .../Microsoft.DotNet.ProjectJsonMigration.Tests/Rules/GivenThatIWantToMigrateBuildOptions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,27 @@ public void Migrating_empty_buildOptions_populates_only_Compile_and_EmbeddedReso
mockProj.Items.First(i => i.ItemType == "EmbeddedResource").Exclude.Should().BeEmpty();
}

[Fact]
public void Migrating_web_project_without_custom_sources_or_resources_does_not_emit_compile_and_embeddedResource()
{
var mockProj = RunBuildOptionsRuleOnPj(@"
{
""buildOptions"": {
""emitEntryPoint"": true
},
""dependencies"": {
""Microsoft.AspNetCore.Mvc"" : {
""version"": ""1.0.0""
}
},
""frameworks"": {
""netcoreapp1.0"": {}
}
}");

mockProj.Items.Count().Should().Be(0);
}

[Fact]
public void Migrating_EmitEntryPoint_true_populates_OutputType_field()
{
Expand Down
65 changes: 40 additions & 25 deletions test/dotnet-migrate.Tests/GivenThatIWantToMigrateTestApps.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class GivenThatIWantToMigrateTestApps : TestBase
[InlineData("TestAppWithRuntimeOptions")]
[InlineData("TestAppWithContents")]
[InlineData("AppWithAssemblyInfo")]
public void It_migrates_apps(string projectName)
public void ItMigratesApps(string projectName)
{
var projectDirectory = TestAssetsManager.CreateTestInstance(projectName, identifier: projectName)
.WithLockFiles()
Expand Down Expand Up @@ -50,7 +50,7 @@ public void It_migrates_apps(string projectName)
}

[Fact]
public void It_migrates_signed_apps()
public void ItMigratesSignedApps()
{
var projectDirectory = TestAssetsManager.CreateTestInstance("TestAppWithSigning").WithLockFiles().Path;

Expand All @@ -74,7 +74,7 @@ public void It_migrates_signed_apps()
}

[Fact]
public void It_migrates_dotnet_new_console_with_identical_outputs()
public void ItMigratesDotnetNewConsoleWithIdenticalOutputs()
{
var testInstance = TestAssetsManager
.CreateTestInstance("ProjectJsonConsoleTemplate");
Expand All @@ -96,8 +96,8 @@ public void It_migrates_dotnet_new_console_with_identical_outputs()
VerifyAllMSBuildOutputsRunnable(projectDirectory);
}

[Fact(Skip="Final tools version missing.")]
public void It_migrates_old_dotnet_new_web_without_tools_with_outputs_containing_project_json_outputs()
[Fact]
public void ItMigratesOldDotnetNewWebWithoutToolsWithOutputsContainingProjectJsonOutputs()
{
var testInstance = TestAssetsManager
.CreateTestInstance("ProjectJsonWebTemplate")
Expand All @@ -108,10 +108,7 @@ public void It_migrates_old_dotnet_new_web_without_tools_with_outputs_containing
var globalDirectory = Path.Combine(projectDirectory, "..");
var projectJsonFile = Path.Combine(projectDirectory, "project.json");

WriteGlobalJson(globalDirectory);
var projectJson = JObject.Parse(File.ReadAllText(projectJsonFile));
projectJson.Remove("tools");
File.WriteAllText(projectJsonFile, projectJson.ToString());
WriteGlobalJson(globalDirectory);

var outputComparisonData = GetComparisonData(projectDirectory);

Expand All @@ -126,9 +123,27 @@ public void It_migrates_old_dotnet_new_web_without_tools_with_outputs_containing
outputsIdentical.Should().BeTrue();
}

public void ItAddsMicrosoftNetWebSdkToTheSdkAttributeOfAWebApp()
{
var testInstance = TestAssetsManager
.CreateTestInstance("ProjectJsonWebTemplate")
.WithLockFiles();

var projectDirectory = testInstance.Path;

var globalDirectory = Path.Combine(projectDirectory, "..");
var projectJsonFile = Path.Combine(projectDirectory, "project.json");

MigrateProject(new [] { projectDirectory });

var csProj = Path.Combine(projectDirectory, $"{new DirectoryInfo(projectDirectory).Name}.csproj");

File.ReadAllText(csProj).Should().Contain(@"Sdk=""Microsoft.NET.Sdk.Web""");
}

[Theory]
[InlineData("TestLibraryWithTwoFrameworks")]
public void It_migrates_projects_with_multiple_TFMs(string projectName)
public void ItMigratesProjectsWithMultipleTFMs(string projectName)
{
var projectDirectory =
TestAssetsManager.CreateTestInstance(projectName, identifier: projectName).WithLockFiles().Path;
Expand All @@ -150,7 +165,7 @@ public void It_migrates_projects_with_multiple_TFMs(string projectName)
[InlineData("TestAppWithLibrary/TestLibrary")]
[InlineData("TestLibraryWithAnalyzer")]
[InlineData("PJTestLibraryWithConfiguration")]
public void It_migrates_a_library(string projectName)
public void ItMigratesALibrary(string projectName)
{
var projectDirectory =
TestAssetsManager.CreateTestInstance(projectName, identifier: projectName).WithLockFiles().Path;
Expand All @@ -174,7 +189,7 @@ public void It_migrates_a_library(string projectName)
[InlineData("ProjectC", "ProjectC,ProjectD,ProjectE")]
[InlineData("ProjectD", "ProjectD")]
[InlineData("ProjectE", "ProjectE")]
public void It_migrates_root_project_and_references(string projectName, string expectedProjects)
public void ItMigratesRootProjectAndReferences(string projectName, string expectedProjects)
{
var projectDirectory =
TestAssetsManager.CreateTestInstance("TestAppDependencyGraph", callingMethod: $"{projectName}.RefsTest").Path;
Expand All @@ -192,7 +207,7 @@ public void It_migrates_root_project_and_references(string projectName, string e
[InlineData("ProjectC")]
[InlineData("ProjectD")]
[InlineData("ProjectE")]
public void It_migrates_root_project_and_skips_references(string projectName)
public void ItMigratesRootProjectAndSkipsReferences(string projectName)
{
var projectDirectory =
TestAssetsManager.CreateTestInstance("TestAppDependencyGraph", callingMethod: $"{projectName}.SkipRefsTest").Path;
Expand All @@ -205,7 +220,7 @@ public void It_migrates_root_project_and_skips_references(string projectName)
[Theory]
[InlineData(true)]
[InlineData(false)]
public void It_migrates_all_projects_in_given_directory(bool skipRefs)
public void ItMigratesAllProjectsInGivenDirectory(bool skipRefs)
{
var projectDirectory = TestAssetsManager.CreateTestInstance("TestAppDependencyGraph", callingMethod: $"MigrateDirectory.SkipRefs.{skipRefs}").Path;

Expand All @@ -224,7 +239,7 @@ public void It_migrates_all_projects_in_given_directory(bool skipRefs)
}

[Fact]
public void It_migrates_given_project_json()
public void ItMigratesGivenProjectJson()
{
var projectDirectory = TestAssetsManager.CreateTestInstance("TestAppDependencyGraph").Path;

Expand All @@ -239,7 +254,7 @@ public void It_migrates_given_project_json()

[Fact]
// regression test for https://github.com/dotnet/cli/issues/4269
public void It_migrates_and_builds_P2P_references()
public void ItMigratesAndBuildsP2PReferences()
{
var assetsDir = TestAssetsManager.CreateTestInstance("TestAppDependencyGraph").WithLockFiles().Path;

Expand Down Expand Up @@ -269,7 +284,7 @@ public void It_migrates_and_builds_P2P_references()
[Theory]
[InlineData("src", "ProjectH")]
[InlineData("src with spaces", "ProjectJ")]
public void It_migrates_and_builds_projects_in_global_json(string path, string projectName)
public void ItMigratesAndBuildsProjectsInGlobalJson(string path, string projectName)
{
var assetsDir = TestAssetsManager.CreateTestInstance(Path.Combine("TestAppDependencyGraph", "ProjectsWithGlobalJson"),
callingMethod: $"ProjectsWithGlobalJson.{projectName}")
Expand Down Expand Up @@ -306,7 +321,7 @@ public void It_migrates_and_builds_projects_in_global_json(string path, string p
[Theory]
[InlineData(true)]
[InlineData(false)]
public void Migration_outputs_error_when_no_projects_found(bool useGlobalJson)
public void MigrationOutputsErrorWhenNoProjectsFound(bool useGlobalJson)
{
var projectDirectory = TestAssetsManager.CreateTestDirectory("Migration_outputs_error_when_no_projects_found");

Expand Down Expand Up @@ -353,7 +368,7 @@ public void Migration_outputs_error_when_no_projects_found(bool useGlobalJson)
}

[Fact]
public void It_migrates_and_publishes_projects_with_runtimes()
public void ItMigratesAndPublishesProjectsWithRuntimes()
{
var projectName = "PJTestAppSimple";
var projectDirectory = TestAssetsManager
Expand All @@ -369,7 +384,7 @@ public void It_migrates_and_publishes_projects_with_runtimes()
[WindowsOnlyTheory]
[InlineData("DesktopTestProjects", "AutoAddDesktopReferencesDuringMigrate", true)]
[InlineData("TestProjects", "PJTestAppSimple", false)]
public void It_auto_add_desktop_references_during_migrate(string testGroup, string projectName, bool isDesktopApp)
public void ItAutoAddDesktopReferencesDuringMigrate(string testGroup, string projectName, bool isDesktopApp)
{
var runtime = DotnetLegacyRuntimeIdentifiers.InferLegacyRestoreRuntimeIdentifier();
var testAssetManager = GetTestGroupTestAssetsManager(testGroup);
Expand All @@ -384,7 +399,7 @@ public void It_auto_add_desktop_references_during_migrate(string testGroup, stri
}

[Fact]
public void It_builds_a_migrated_app_with_a_indirect_dependency()
public void ItBuildsAMigratedAppWithAnIndirectDependency()
{
const string projectName = "ProjectA";
var solutionDirectory =
Expand All @@ -399,7 +414,7 @@ public void It_builds_a_migrated_app_with_a_indirect_dependency()
}

[Fact]
public void It_migrates_project_with_output_name()
public void ItMigratesProjectWithOutputName()
{
string projectName = "AppWithOutputAssemblyName";
string expectedOutputName = "MyApp";
Expand All @@ -426,18 +441,18 @@ public void It_migrates_project_with_output_name()
[Theory]
[InlineData("LibraryWithoutNetStandardLibRef")]
[InlineData("LibraryWithNetStandardLibRef")]
public void It_migrates_and_builds_library(string projectName)
public void ItMigratesAndBuildsLibrary(string projectName)
{
var projectDirectory = TestAssetsManager.CreateTestInstance(projectName,
callingMethod: $"{nameof(It_migrates_and_builds_library)}-projectName").Path;
callingMethod: $"{nameof(ItMigratesAndBuildsLibrary)}-projectName").Path;

MigrateProject(projectDirectory);
Restore(projectDirectory, projectName);
BuildMSBuild(projectDirectory, projectName);
}

[Fact]
public void It_fails_gracefully_when_migrating_app_with_missing_dependency()
public void ItFailsGracefullyWhenMigratingAppWithMissingDependency()
{
string projectName = "MigrateAppWithMissingDep";
var projectDirectory = Path.Combine(GetTestGroupTestAssetsManager("NonRestoredTestProjects").CreateTestInstance(projectName).Path, "MyApp");
Expand Down

0 comments on commit 8cec61c

Please sign in to comment.