Skip to content

Conversation

@dfields-msft
Copy link
Contributor

Fixes #1010 by calling Catch2's reportFatal() in the std::terminate handler so that failure logging is generated before the process exits.

@dfields-msft dfields-msft mentioned this pull request Sep 1, 2021
@kennykerr
Copy link
Collaborator

Thanks David! Why don't you add the noexcept back to the guid constructor that I removed here:

https://github.com/microsoft/cppwinrt/pull/1009/files

That will verify whether the CI build fails visibly or silently. Once that has been confirmed, just remove the noexcept again and it should be green again.

@kennykerr
Copy link
Collaborator

The log captures the failure but it doesn't fail the build itself.

image

@dfields-msft
Copy link
Contributor Author

It's worse than that - even normal test failures don't cause the pipeline to fail (see my latest commit in this PR and the corresponding run). I've sent email to the team to discuss.

@dfields-msft dfields-msft force-pushed the user/dfields/report-fatal branch from 4fc1201 to 7afc84c Compare September 7, 2021 22:25
@dfields-msft dfields-msft force-pushed the user/dfields/report-fatal branch from 7afc84c to fd0b747 Compare September 7, 2021 22:50
@dfields-msft
Copy link
Contributor Author

Added some changes to reroute output to stderr when the test executable does not exit cleanly, and validated that this fails the pipeline with the noexcept in place as expected:
image

Copy link
Collaborator

@kennykerr kennykerr left a comment

Choose a reason for hiding this comment

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

Thanks!

@dfields-msft
Copy link
Contributor Author

Logged #1012 on an unrelated test failure that passed on rerun (also passed on both x86 runs); this may have been masked for some time due to the lack of failure reporting in the build pipeline until now.

@kennykerr kennykerr merged commit 3aa0339 into master Sep 8, 2021
@kennykerr kennykerr deleted the user/dfields/report-fatal branch September 8, 2021 00:09
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.

Unit tests that terminate abnormally are not considered as failures

3 participants