-
Notifications
You must be signed in to change notification settings - Fork 230
Unskip cohosting diagnostics tests #11541
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
Unskip cohosting diagnostics tests #11541
Conversation
I suspect this will make some of the skipped tests fail, which will help track down issues
|
|
||
| var projectBasePath = Path.GetDirectoryName(razorDocument.Project.FilePath); | ||
| var relativeDocumentPath = razorDocument.FilePath[projectBasePath.Length..].TrimStart('/', '\\'); | ||
| hintName = RazorSourceGenerator.GetIdentifierFromPath(relativeDocumentPath); |
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.
@chsienki Do you think this is reliable enough?
Alternative would be to see if we can expose a method through our EA that computes the path, including the generator Guid or whatever goo is put in there, except I'm not sure if thats possible in devenv anyway.
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.
Hmm, so the code in roslyn that generates the full path to the generated file is pretty simple, but not public https://github.com/dotnet/roslyn/blob/e7f61a2c04c6964b0cb431a15c77dd58da7e453a/src/Compilers/Core/Portable/SourceGeneration/GeneratorDriver.cs#L440 and I don't think we're likely to want to depend on it.
Given that I think this approach seems safe enough?
I'm not sure the context in which is this is called. Presumably TextDocument is the actual .razor document? I also assume at this point we don't have access to any of the razor-specific stuff, so it's just sort of an opaque file with some text in it?
I'm just trying to wonder if we can centralize the hint name stuff somewhere so that it's a property of a razor document which the generator just uses as-is. Then we don't need to 'go to' the generator to get it. It probably doesn't make much difference though.
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.
I also assume at this point we don't have access to any of the razor-specific stuff, so it's just sort of an opaque file with some text in it?
Yeah, this case specifically is complicated by being in devenv, not OOP, so razorDocument is just an additional file in a Roslyn 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.
I'm just trying to wonder if we can centralize the hint name stuff somewhere so that it's a property of a razor document which the generator just uses as-is. Then we don't need to 'go to' the generator to get it. It probably doesn't make much difference though.
This also brings up TargetPath stuff we've talked about too. That is essentially what this is producing, and it matches what we do in tests for the generated .editorconfig, but we've talked about moving that into the generator too. Would be nice if we had one method to call that did the computation in future.
...Studio.LanguageServices.Razor/LanguageClient/Cohost/CohostDocumentPullDiagnosticsEndpoint.cs
Outdated
Show resolved
Hide resolved
ryzngard
left a comment
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.
I think this is fine + questions you had for Chris
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
Step one of The Great Unskippening!™
Part of #10693
It is no longer simply possible to use string manipulation to get a generated C# document path from a Razor document path. The alternative options for what is available differ depending on which process code is running in. Yay! :)