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

Swallow MSDTC availability exceptions for CI test reliability #77023

Merged
merged 1 commit into from
Oct 18, 2022
Merged

Swallow MSDTC availability exceptions for CI test reliability #77023

merged 1 commit into from
Oct 18, 2022

Conversation

roji
Copy link
Member

@roji roji commented Oct 13, 2022 •
edited
Loading

Fixes #76836

/cc @ajcvickers

@ghost ghost assigned roji Oct 13, 2022
@ghost
Copy link

ghost commented Oct 13, 2022

Tagging subscribers to this area: @roji
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #76836

Author: roji
Assignees: -
Labels:

area-System.Transactions
Milestone: -

roji
roji commented Oct 13, 2022
View reviewed changes
src/libraries/System.Transactions.Local/tests/OleTxTests.cs
// We don't want this to make our CI flaky, so we swallow the exception and skip all subsequent tests.
s_isTestSuiteDisabled = true;

return;
Copy link
Member Author

Choose a reason for hiding this comment

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

@stephentoub do we have a standard way to detect CI, so that this can continue throwing when run locally (e.g. GetEnvironmentVariable("CI") or something)?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not aware of a method we call anywhere to ask whether we're running in CI, but we do have an attribute we apply to suppress a test when in CI:

[Trait(XunitConstants.Category, XunitConstants.IgnoreForCI)]

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I'll take a look at how that's implemented.

Copy link
Member

Choose a reason for hiding this comment

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

Here's the implementation you're looking for:

https://github.com/dotnet/arcade/blob/main/src/Microsoft.DotNet.XUnitExtensions/src/Discoverers/SkipOnCIDiscoverer.cs#L19-L21

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you!

@roji roji mentioned this pull request Oct 18, 2022
Closed
@BruceForstall
Copy link
Member

@roji Is there anything blocking this from being reviewed/merged? The failure noise in AzDO pipelines from these tests is quite annoying.

@roji
Copy link
Member Author

roji commented Oct 18, 2022

Sorry, I've been busy with other things - will give this my priority.

@roji roji merged commit fdcbc7b into dotnet:main Oct 18, 2022
@roji roji deleted the IgnoreDTCFailures branch October 18, 2022 22:02
@ghost ghost locked as resolved and limited conversation to collaborators Nov 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@stephentoub stephentoub stephentoub left review comments

@jkoritzinsky jkoritzinsky jkoritzinsky left review comments

@ajcvickers ajcvickers ajcvickers approved these changes

Assignees

@roji roji

Labels
area-System.Transactions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Test failure in System.Transactions.Tests.OleTxTests.* tests
5 participants
@roji @BruceForstall @ajcvickers @jkoritzinsky @stephentoub