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

Some QoL improvements for diag tests #1204

Merged
merged 3 commits into from
Aug 3, 2022

Conversation

asklar
Copy link
Member

@asklar asklar commented Jun 15, 2022

  • Fixes DiagnosticTests! There were a few non-passing tests so I debugged into it and saw that we're failing because in some codepaths we didn't have an assembly name or version. Passing these requires a context options object.
  • DiagnosticTests can now pass a context with options, so we can remove special casing we had for cswinrt generator when running inside a test host
  • When tests failed, the inner exception was not being captured, so the error just said "...But found:" and no additional information about the exception in the generator was provided
  • Setting ExitCode upon error, which should cause builds to fail when the generator fails to produce. Fixes Build succeeds even when generator fails #1202
  • Fixes some typos

image

@asklar asklar requested a review from j0shuams June 15, 2022 23:50
Copy link
Member

@manodasanW manodasanW left a comment

Choose a reason for hiding this comment

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

Thanks @asklar for these changes.

src/Tests/DiagnosticTests/UnitTesting.cs Show resolved Hide resolved
src/Tests/DiagnosticTests/Helpers.cs Outdated Show resolved Hide resolved
@@ -154,6 +152,7 @@ public void Generate()
{
Logger.Log("Exiting early -- found errors in authored runtime component.");
Logger.Close();
Environment.ExitCode = -1;
Copy link
Member

Choose a reason for hiding this comment

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

Didn't realize we can do this, does this make the build fail out and how does it look like when it does?

Copy link
Contributor

@j0shuams j0shuams left a comment

Choose a reason for hiding this comment

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

Looks good to me, just needs some feedback on the comments

@asklar asklar force-pushed the diagtestContextOptions branch from 7b13929 to e98230b Compare August 3, 2022 17:37
@j0shuams
Copy link
Contributor

j0shuams commented Aug 3, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@j0shuams j0shuams merged commit 2a3dc38 into microsoft:master Aug 3, 2022
@asklar asklar deleted the diagtestContextOptions branch August 3, 2022 21:44
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.

Build succeeds even when generator fails
3 participants