-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Listen to build manager events to know when build is complete #64366
Listen to build manager events to know when build is complete #64366
Conversation
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/SolutionExplorerInProcess.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/SolutionExplorerInProcess.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/SolutionExplorerInProcess.cs
Show resolved
Hide resolved
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/SolutionExplorerInProcess.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/SolutionExplorerInProcess.cs
Outdated
Show resolved
Hide resolved
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/SolutionExplorerInProcess.cs
Show resolved
Hide resolved
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/SolutionExplorerInProcess.cs
Outdated
Show resolved
Hide resolved
162e09e
to
048cc80
Compare
// The build summary line should be second to last in the output window | ||
return lines[^2].Extent.GetText(); | ||
// Find the build summary line | ||
for (var index = lines.Count - 1; index > 0; index--) |
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.
Found that this was returning "Elapsed time" instead of the build summary.
src/VisualStudio/IntegrationTest/New.IntegrationTests/InProcess/SolutionExplorerInProcess.cs
Outdated
Show resolved
Hide resolved
@jasonmalinowski This is ready for another review. I believe Shen is taking care of the newly failing integration tests in another PR. |
for (var index = lines.Count - 1; index >= 0; index--) | ||
{ | ||
var lineText = lines[index].Extent.GetText(); | ||
if (lineText.StartsWith("========== Build:")) |
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.
if (lineText.StartsWith("========== Build:")) | |
if (lineText.StartsWith("==========")) |
Since that won't depend on localization anymore.
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 agree, but the reality is that every test that uses this method immediately Asserts the returned string equals an English build summary. Probably long term we should capture the success, failed, and canceled values returned by the UpdateSolutionEvents Done and return those instead of the build summary.
Fixes CSharpBuild.BuildProject integration test when running against IntPreview builds of VS.