Skip to content
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

Run all cohosting tests with FUSE on and off #11330

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

davidwengier
Copy link
Contributor

@davidwengier davidwengier commented Dec 23, 2024

Fixes https://github.com/orgs/dotnet/projects/197/views/8?query=is%3Aopen+sort%3Aupdated-desc&pane=issue&itemId=85450324

Review commit and a time is recommended. The second last commit is the only one you need to worry about @dotnet/razor-compiler, it fixes various scenarios around using directives, as previously the code-gen was:

using System

#line hidden
;

but having any #line hidden in a using directive means Roslyn won't offer to add them, so this just updates the code-gen to move the semi-colon to:

using System

;
#line hidden

@davidwengier davidwengier requested review from a team as code owners December 23, 2024 02:09
Copy link
Member

@jjonescz jjonescz left a comment

Choose a reason for hiding this comment

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

last commit LGTM


public override Task<RunSummary> RunAsync(IMessageSink diagnosticMessageSink, IMessageBus messageBus, object[] constructorArguments, ExceptionAggregator aggregator, CancellationTokenSource cancellationTokenSource)
{
Debug.Assert(constructorArguments.Length >= 1 && constructorArguments[0] is FuseTestContext, $"{TestMethod.TestClass.Class.Name}.{TestMethod.Method.Name} uses a formatting test attribute in a class without a FuseTestContext parameter?");
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we know that a formatting test attribute is getting used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Strictly speaking, we don't, but instances of this test case are only created by the discoverer for those attributes. If we could actually know for sure, then this assert wouldn't be needed.

@davidwengier
Copy link
Contributor Author

@dotnet/razor-compiler for 2nd review, in case anyone is back from holidays :)

@davidwengier
Copy link
Contributor Author

davidwengier commented Jan 6, 2025

@dotnet/razor-compiler for 2nd review, in case anyone is back from holidays :)

I was a bit early with this, but hopefully you're back now @333fred and/or @chsienki ?

You only need to look at ec8615f (#11330)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants