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

Ignore virtual files #3592

Merged
merged 2 commits into from Feb 25, 2020
Merged

Conversation

@ryanbrandenburg
Copy link
Contributor

ryanbrandenburg commented Feb 20, 2020

Fixes dotnet/aspnetcore#18927.

This seemed to require editing a lot of test files to exclude the Blazor project from running other tests, if anyone on the O# side has a better way to handle that I'm open.

CC @NTaylorMullen, @ajaybhargavb

@NTaylorMullen NTaylorMullen requested review from filipw and JoeRobich Feb 20, 2020
@ryanbrandenburg

This comment has been minimized.

Copy link
Contributor Author

ryanbrandenburg commented Feb 20, 2020

@JoeRobich / @filipw I'm not 100% sure this is the correct fix, maybe you could correct my thinking (or convince me I'm already right).

Repro steps:

  1. dotnet new blazorserver
  2. In Index.razor add the line @DoesNotExist.
  3. Watch the Problems pane, you should see an entry for Index.razor_virtual.cs flash briefly (occasionally it stays permanently). If you didn't see it try again, it should repro each time.

When I track this down I can find requests to _validateDocument for the Index.cshtml_virtual.cs file, which has returned a result for Index.cshtml, even though the request was for _virtual.cs.

Hopefully that all makes sense, I can go into more detail offline if you need more context or a demo.

@JoeRobich

This comment has been minimized.

Copy link
Contributor

JoeRobich commented Feb 20, 2020

@ryanbrandenburg I tried the step you listed and this is what I see in the latest release 1.21.12. I do see a reported problem, but it is for Index.razor.

image

@JoeRobich

This comment has been minimized.

Copy link
Contributor

JoeRobich commented Feb 20, 2020

@ryanbrandenburg I spoke too soon. I see what you are talking about.
image

@ryanbrandenburg ryanbrandenburg force-pushed the ryanbrandenburg:rybrande/VirtualFiles branch from c35ca2e to e6baffa Feb 25, 2020
@codecov

This comment has been minimized.

Copy link

codecov bot commented Feb 25, 2020

Codecov Report

Merging #3592 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #3592   +/-   ##
======================================
  Coverage    89.8%   89.8%           
======================================
  Files          59      59           
  Lines        1589    1589           
  Branches       89      89           
======================================
  Hits         1427    1427           
  Misses        151     151           
  Partials       11      11
Flag Coverage Δ
#integration ?
#unit 89.8% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ebb473...e78397c. Read the comment docs.

@ryanbrandenburg ryanbrandenburg force-pushed the ryanbrandenburg:rybrande/VirtualFiles branch from e6baffa to 8cb1662 Feb 25, 2020
@ryanbrandenburg

This comment has been minimized.

Copy link
Contributor Author

ryanbrandenburg commented Feb 25, 2020

@JoeRobich, I've updated it with your feedback but it seems I don't have permissions to merge.

@JoeRobich JoeRobich merged commit ce7433c into OmniSharp:master Feb 25, 2020
5 checks passed
5 checks passed
codecov/patch Coverage not affected when comparing 1ebb473...e78397c
Details
codecov/project 89.8% remains the same compared to 1ebb473
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.
Details
security/snyk (david-driscoll) No manifest changes detected
Details
@JoeRobich

This comment has been minimized.

Copy link
Contributor

JoeRobich commented Feb 25, 2020

Thanks @ryanbrandenburg !

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

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.