Override default Arcade Xunit configuration by cdmihai · Pull Request #5302 · dotnet/msbuild

cdmihai · 2020-04-21T00:02:20Z

No description provided.

cdmihai · 2020-04-21T01:34:49Z

-
+
+    <!-- Makes xunit print all test names in stdout -->
+    <XunitOptions>$(XunitOptions) -diagnostics</XunitOptions>


Preventing arcade from overriding the xunit json makes this line useless. But I guess it doesn't hurt, in case other random things overwrite the json in the future.

cdmihai · 2020-04-21T01:36:10Z

    <PackageOutputPath Condition="'$(IsVisualStudioInsertionPackage)' == 'true'">$(DevDivPackagesDir)</PackageOutputPath>
+
+    <!-- Arcade sdk also carries an xunit.runner.json which sometimes overrides the one in this repo. Assign a value to the arcade properties XUnitDesktopSettingsFile and XUnitCoreSettingsFile to prevent the arcade version of the file being added. -->
+    <XUnitDesktopSettingsFile>$(MSBuildThisFileDirectory)Shared\UnitTests\xunit.runner.json</XUnitDesktopSettingsFile>


@Forgind AFAIK you were investigating test hangs. If you copy this diff, I have a suspicion long running tests will start getting reported.

Also, from what I read, the long running tests option does not make xunit kill long running tests, it just reports them.

What's odd is that it, at least sometimes, already works. If you look at the log I attached to the issue you mentioned, it reports that a test was taking a long time. Then it reports it again. And again. And a number of other times. But that made it easy for me to insert an explicit timeout on that test, so that test should be slightly less of a problem in the future.

What's odd is that it, at least sometimes, already works.

I'd guess it's nondeterministic whether the arcade xunit json overwrites ours, at least that's what the logs suggest.

cdmihai · 2020-04-21T01:42:22Z

 # Visual Studio 15
 VisualStudioVersion = 15.0.27004.2009
 MinimumVisualStudioVersion = 10.0.40219.1
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{4900B3B8-4310-4D5B-B1F7-2FDF9199765F}"


This snippet is in MSBuild.sln but not in MSBuild.dev.sln. So I copied it over. No idea what it does, but why not? :)

It makes these things appear in the sidebar:

That's the first degree effect, but does it lead to those files getting picked up by anything else in VS? Forever a mystery.

I don't think it should but I'm not willing to say it with that much confidence.

rainersigwald

Thought about saying we should remove the xunit.runner.json items from the individual test projects but . . . nah.

rainersigwald · 2020-04-22T22:20:21Z

 # Visual Studio 15
 VisualStudioVersion = 15.0.27004.2009
 MinimumVisualStudioVersion = 10.0.40219.1
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{4900B3B8-4310-4D5B-B1F7-2FDF9199765F}"


It makes these things appear in the sidebar:

rainersigwald · 2020-04-22T22:21:03Z


    <PackageOutputPath Condition="'$(IsVisualStudioInsertionPackage)' == 'true'">$(DevDivPackagesDir)</PackageOutputPath>
+
+    <!-- Arcade sdk also carries an xunit.runner.json which sometimes overrides the one in this repo. Assign a value to the arcade properties XUnitDesktopSettingsFile and XUnitCoreSettingsFile to prevent the arcade version of the file being added. -->


can you put some color on "sometimes"? Should I go yell at them?

To be honest I didn't understand why it only happens on Microsoft.Build.Engine.UnitTests.csproj and not the rest. When I compare the logs to other projects, it all looks the same until the copy tasks does otherwise on that one project. So I just went with this fix.

rainersigwald · 2020-04-22T22:21:37Z

    <PackageOutputPath Condition="'$(IsVisualStudioInsertionPackage)' == 'true'">$(DevDivPackagesDir)</PackageOutputPath>
+
+    <!-- Arcade sdk also carries an xunit.runner.json which sometimes overrides the one in this repo. Assign a value to the arcade properties XUnitDesktopSettingsFile and XUnitCoreSettingsFile to prevent the arcade version of the file being added. -->
+    <XUnitDesktopSettingsFile>$(MSBuildThisFileDirectory)Shared\UnitTests\xunit.runner.json</XUnitDesktopSettingsFile>


* LOC CHECKIN | microsoft/msbuild vs16.6 | 20200420 (#5299) * Final branding_16.6 (#5273) * merge * Enable handshake timeout on Mono (#5318) I am seeing consistent test hangs on macOS Mono. A node is getting into a bad state and is failing to respond to handshakes. While I do not yet understand the root cause, it is clear that having a timeout on the handshake operation mitigates the issue. The test is now failing but does not hang, saving developer time as well as test pipeline resources. * Revert "Reverted and rebuilt for localizable strings." (#5246) This adds back support for logging an error when a task returns false without logging an error. This was originally added in #4940 but was reverted because of multiple difficulties. * Changed error code * Add escape hatch * Fix typo * Filtering for duplicate content files with referencing projects (#4931) * Updating content filtering based on content copying changes * Add a flag that is enabled by default on Core; otherwise disabled by default. * Adding Shutdown Message to Worker Nodes (#5262) * Changed where Trace is being called and removed old functionality. * Updating .NET Core branding to .NET (#5286) * Override default Arcade Xunit configuration (#5302) * Update Directory.Build.targets * prevent arcade from injecting its own xunit file * Don't log @(ReferencePath) at the end of ImplicitlyExpandDesignTimeFacades (#5317) The actual ItemGroups inside the target already do a good job of logging exactly what items were added and/or removed and in what order and with what metadata. Emitting an extra low-pri message which is unstructured here just adds noise, slows the builds down, wastes binlog space and is otherwise redundant. * Compute hashes in parallel in GetFileHash (#5303) * Compute hashes in parallel. This scales better for larger number of files. * Use a dedicated write lock * Allow disabling logging of task parameters and item metadata (#5268) This enables fine-grained control over whether: * to log log each parameter (whether input or output) * or whether to log item metadata for each ITaskItem[] parameter. When LogTaskInputs is set the default behavior is still to log all parameters and all item metadata for ITaskItem[] parameters. Since this is very verbose and hurts performance without adding any useful information it is valuable to be able to turn this logging off in certain situations. This approach allows controlling logging via setting simple properties or environment variables. I've identified the specific tasks and parameters that we want to restrict logging for that would give us the most gains without losing any significant useful info: https://github.com/KirillOsenkov/MSBuildStructuredLog/wiki/Task-Parameter-Logging * Update dependencies from https://github.com/dotnet/arcade build 20200430.5 (#5325) - Microsoft.DotNet.Arcade.Sdk: 1.0.0-beta.20221.2 -> 1.0.0-beta.20230.5 Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> * Use environment variable for handshake Resolves #4961 (#5196) * Use environment variable for handshake Resolves #4961 * Combine means of hashing * Support transitive project references in static graph (#5326) Transitive project references are a thing now. Added support to static graph, so that buildxl and qb can avoid adding the transitive refs. This PR is independent of #5222. Ideally, review this one first, as QB has a stronger dependency on this PR than on #5222. Design - transitive references are opt-in, per project evaluation - once a project opts-in, transitivity is applied for all ProjectReference items - a project opt-ins by setting the property AddTransitiveProjectReferencesInStaticGraph to true. The sdk does this automatically in Microsoft.Managed.After.Targets. - interaction with crosstargeting: transitive refs are added only to inner builds, not the outer builds. This mimics vanilla msbuild. Co-authored-by: Rainer Sigwald <raines@microsoft.com> Co-authored-by: Martin Chromecek (Moravia IT) <v-chmart@microsoft.com> Co-authored-by: Ladi Prosek <laprosek@microsoft.com> Co-authored-by: Sarah Oslund <sfoslund@microsoft.com> Co-authored-by: Ben Villalobos <villalobosb93@gmail.com> Co-authored-by: Mihai Codoban <codobanmihai@gmail.com> Co-authored-by: Kirill Osenkov <KirillOsenkov@users.noreply.github.com> Co-authored-by: Pranav K <prkrishn@hotmail.com> Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com> Co-authored-by: Rainer Sigwald <raines@microsoft.com>

Update Directory.Build.targets

dc820fd

cdmihai changed the title ~~Xunit logging~~ [WIP] Xunit logging Apr 21, 2020

prevent arcade from injecting its own xunit file

018d5e4

cdmihai force-pushed the Xunit-logging branch from d817f71 to 018d5e4 Compare April 21, 2020 01:31

cdmihai commented Apr 21, 2020

View reviewed changes

No idea what this does but maybe it does something good.

3cc3cd5

cdmihai commented Apr 21, 2020

View reviewed changes

cdmihai changed the title ~~[WIP] Xunit logging~~ Xunit logging Apr 22, 2020

rainersigwald approved these changes Apr 22, 2020

View reviewed changes

rainersigwald changed the title ~~Xunit logging~~ Override default Arcade Xunit configuration Apr 23, 2020

Forgind merged commit 0b9b0b9 into master Apr 28, 2020

Forgind deleted the Xunit-logging branch December 6, 2021 17:49



		<!-- Makes xunit print all test names in stdout -->
		<XunitOptions>$(XunitOptions) -diagnostics</XunitOptions>


		<PackageOutputPath Condition="'$(IsVisualStudioInsertionPackage)' == 'true'">$(DevDivPackagesDir)</PackageOutputPath>

		<!-- Arcade sdk also carries an xunit.runner.json which sometimes overrides the one in this repo. Assign a value to the arcade properties XUnitDesktopSettingsFile and XUnitCoreSettingsFile to prevent the arcade version of the file being added. -->

Uh oh!

Conversation

cdmihai commented Apr 21, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rainersigwald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants