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.

Sign up for GitHub

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

Jump to bottom

Add cross-plat tests to Razor.slim.sln #2548

Closed
wants to merge 4 commits into from
Closed

Add cross-plat tests to Razor.slim.sln #2548

wants to merge 4 commits into from

Conversation

NTaylorMullen
Copy link
Contributor

  • This ensures that these language server tests are running cross-plat.
  • Initially I expect this test to fail with several cross-plat related test breaks. We'll update the PR to get them all building

@ajaybhargavb
Copy link
Contributor

There are a bunch of failing tests. You can ignore the failed formatting tests - they will be fixed once my PR is merged. Other failures are legit though and needs investigation.

This was referenced Sep 24, 2020
Closed
Closed
ryanbrandenburg
ryanbrandenburg approved these changes Sep 24, 2020
View reviewed changes
Copy link
Contributor

@ryanbrandenburg ryanbrandenburg left a comment

Choose a reason for hiding this comment

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

LGTM once the tests pass.

@ajaybhargavb
Copy link
Contributor

@NTaylorMullen can we skip the tests that are failing(file an issue to unskip) and merge this? It is difficult to verify crossplat correctness with new tests I am adding with formatting

@NTaylorMullen
Add cross-plat tests to Razor.slim.sln
975d46b 
- This ensures that these language server tests are running cross-plat.
@NTaylorMullen NTaylorMullen force-pushed the nimullen/reenabletests branch from 33740cf to f7718a9 Compare October 5, 2020 17:38
@NTaylorMullen
Copy link
Contributor Author

There are a bunch of failing tests. You can ignore the failed formatting tests - they will be fixed once my PR is merged. Other failures are legit though and needs investigation.

Sounds good. I've filed https://github.com/dotnet/aspnetcore/issues/26612 and skipped them here.

@NTaylorMullen NTaylorMullen added the auto-merge Squash merge once all PR checks are complete and reviewers have approved label Oct 5, 2020
@ghost
Copy link

ghost commented Oct 5, 2020

Hello @NTaylorMullen!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@TanayParikh
Copy link
Contributor

@NTaylorMullen looks like there's another error:

src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/MonitorProjectConfigurationFilePathEndpointTest.cs(202,62): error CS1003: (NETCORE_ENGINEERING_TELEMETRY=Build) Syntax error, ',' expected

@NTaylorMullen
Skip and fix broken cross-plat tests.
2f19c45 
These tests will be reenabled as part of dotnet/aspnetcore#26612
@NTaylorMullen NTaylorMullen force-pushed the nimullen/reenabletests branch from f7718a9 to 2f19c45 Compare October 5, 2020 18:03
@TanayParikh
Copy link
Contributor

Linux jobs not going through in the CI, re-triggered.

@TanayParikh TanayParikh closed this Oct 6, 2020
@TanayParikh TanayParikh reopened this Oct 6, 2020
@NTaylorMullen
Copy link
Contributor Author

Somehow the Linux tests are hanging every time. Will have to investigate a dump at some point

@TanayParikh
Copy link
Contributor

Is the branch safe to delete as well?

@NTaylorMullen
Copy link
Contributor Author

Is the branch safe to delete as well?

Leaving it active for now to revisit this. Just wanted to unblock doug

@NTaylorMullen NTaylorMullen deleted the nimullen/reenabletests branch March 23, 2022 23:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TanayParikh TanayParikh TanayParikh approved these changes

@ryanbrandenburg ryanbrandenburg ryanbrandenburg approved these changes

@ajaybhargavb ajaybhargavb Awaiting requested review from ajaybhargavb

Assignees
No one assigned
Labels
auto-merge Squash merge once all PR checks are complete and reviewers have approved
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@NTaylorMullen @ajaybhargavb @TanayParikh @ryanbrandenburg