-
Notifications
You must be signed in to change notification settings - Fork 10.4k
FunctionalTests - Clean up publish of deps.json #29449
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 4 commits
d88a419
456b195
e6d876e
69732f1
048cbc3
dadf5a4
90fe4f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,11 @@ public class GenerateMvcTestManifestTask : Task | |
[Required] | ||
public string ManifestPath { get; set; } | ||
|
||
/// <summary> | ||
/// The path to copy deps files to. | ||
/// </summary> | ||
public string PathToCopyDeps { get; set; } | ||
|
||
/// <summary> | ||
/// A list of content root paths and assembly names to generate the | ||
/// manifest from. | ||
|
@@ -35,12 +40,27 @@ public override bool Execute() | |
{ | ||
using var fileStream = File.Create(ManifestPath); | ||
var output = new Dictionary<string, string>(); | ||
|
||
foreach (var project in Projects) | ||
{ | ||
var contentRoot = project.GetMetadata("ContentRoot"); | ||
var assemblyName = project.GetMetadata("Identity"); | ||
output[assemblyName] = contentRoot; | ||
|
||
// This is only set when publishing | ||
if (string.IsNullOrEmpty(PathToCopyDeps)) | ||
{ | ||
output[assemblyName] = contentRoot; | ||
} | ||
else | ||
{ | ||
// When publishing content root is always the BaseDirectory | ||
output[assemblyName] = "~"; | ||
var depsFile = project.GetMetadata("DepsFile"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💯 This is glorious! |
||
Log.LogMessage("Looking for " + depsFile + ": "+ File.Exists(depsFile)); | ||
if (File.Exists(depsFile)) | ||
{ | ||
File.Copy(depsFile, Path.Combine(PathToCopyDeps, Path.GetFileName(depsFile))); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. One small nit is that it might be better to do this within a target so that we can leverage I think it might be easier to just add these files to the resolvedfilestopublish itemgroup and that way it reuses the same standard insfrastructure. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, yeah that looks better than doing it in the task, updated with doing it in the targets file |
||
} | ||
} | ||
} | ||
|
||
var serializer = new DataContractJsonSerializer(typeof(Dictionary<string, string>), new DataContractJsonSerializerSettings | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,8 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
<Import Project="$(MvcTestingTargets)" Condition="'$(MvcTestingTargets)' != ''" /> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework> | ||
|
||
<DefineConstants Condition="'$(GenerateBaselines)'=='true'">$(DefineConstants);GENERATE_BASELINES</DefineConstants> | ||
<DefineConstants>$(DefineConstants);__RemoveThisBitTo__GENERATE_BASELINES</DefineConstants> | ||
<TestGroupName>Mvc.FunctionalTests</TestGroupName> | ||
<!-- The test asset projects this depends on are not strong-named. --> | ||
<SignAssembly>false</SignAssembly> | ||
|
@@ -18,14 +15,6 @@ | |
<None Include="xunit.runner.json" CopyToOutputDirectory="PreserveNewest" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<!-- For testing the testing infrastructure only --> | ||
<WebApplicationFactoryContentRootAttribute Include="BasicWebSite" AssemblyName="BasicWebsite" ContentRootPath="../../../../WebSites/BasicWebSite" ContentRootTest="BasicWebSite.csproj" Priority="-1" /> | ||
<!-- For testing the testing infrastructure only. This attribute is | ||
incorrect by design to ensure that the infrastructure falls back. --> | ||
<WebApplicationFactoryContentRootAttribute Include="FiltersWebSite" AssemblyName="FiltersWebSite" ContentRootPath="/../WebSites/FiltersWebSite" ContentRootTest="Incorrect.csproj" Priority="-1" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<Reference Include="Microsoft.AspNetCore.Mvc.Testing" /> | ||
<ProjectReference Include="..\..\samples\MvcSandbox\MvcSandbox.csproj" /> | ||
|
@@ -59,30 +48,4 @@ | |
<Reference Include="Microsoft.AspNetCore.TestHost" /> | ||
</ItemGroup> | ||
|
||
<Target Name="PublishAssets" AfterTargets="Publish"> | ||
<ItemGroup> | ||
<_PublishFiles Include="$(ArtifactsBinDir)ApiExplorerWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\ApiExplorerWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)ApplicationModelWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\ApplicationModelWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)BasicWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\BasicWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)ControllersFromServicesWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\ControllersFromServicesWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)CorsWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\CorsWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)ErrorPageMiddlewareWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\ErrorPageMiddlewareWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)FilesWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\FilesWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)FormatterWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\FormatterWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)GenericHostWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\GenericHostWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)HtmlGenerationWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\HtmlGenerationWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)Mvc.RoutingWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\Mvc.RoutingWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)MvcSandbox\$(Configuration)\$(DefaultNetCoreTargetFramework)\MvcSandbox.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)RazorBuildWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\RazorBuildWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)RazorPagesWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\RazorPagesWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)RazorWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\RazorWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)SecurityWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\SecurityWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)SimpleWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\SimpleWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)TagHelpersWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\TagHelpersWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)VersioningWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\VersioningWebSite.deps.json" /> | ||
<_PublishFiles Include="$(ArtifactsBinDir)XmlFormattersWebSite\$(Configuration)\$(DefaultNetCoreTargetFramework)\XmlFormattersWebSite.deps.json" /> | ||
</ItemGroup> | ||
<Copy SourceFiles="@(_PublishFiles)" DestinationFolder="$(PublishDir)" /> | ||
</Target> | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What was the magic behind removing this, we simply don't need it anymore? (We used to need it for application parts and Razor runtime compilation and we use the API within the WAF for something but can't remember where/why) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These weren't here in the MVC Functionals csproj until my last PR, I just added them to copy over the deps file to get them to the publish directory for helix to pickup, that copy logic is now just in the build task, so this is just reverting the hack from the last PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That said, can you think of any reasons why script and link tags would change their rendering output now that they are running in the publish directory instead of the src directory? Is it something to do with how relative url/paths might render, and I just need to update the baselines? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Release/Debug + Environment tag helper There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok I will follow up in a 3rd PR just looking at restoring those 2-3 skipped tests since this one is basically ready to go |
||
</Project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like the UI functional tests need some additional dll/pages, in addition to the deps json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, these switch the application parts for testing V3 and V4 bootstrap I believe. We might be able to take them out of here since we removed bootstrap 3 support I believe, but we'll have to redo this work every time we have two ongoing supported versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So these are fine to just leave for now in the csproj? The tests fail to run without these files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HaoK I see what you did now. It's glorious!!