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

Don't throw from RemoteExecutor on SkipTestExceptions #65105

Merged
merged 1 commit into from
Feb 10, 2022

Conversation

MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Feb 9, 2022

Fixes #63008

Should be reviewed without looking at whitespace changes.

@MihaZupan MihaZupan added this to the 7.0.0 milestone Feb 9, 2022
@MihaZupan MihaZupan requested a review from wfurt February 9, 2022 21:22
@ghost ghost assigned MihaZupan Feb 9, 2022
@ghost
Copy link

ghost commented Feb 9, 2022

Tagging subscribers to this area: @dotnet/ncl, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #63008

Should be reviewed without looking at whitespace changes.

Author: MihaZupan
Assignees: -
Labels:

area-System.Net.Security

Milestone: 7.0.0

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM.

@jkoritzinsky
Copy link
Member

Is there any interest in generalized support for communicating a SkipTestException from a RemoteExecutor callback back to the test process? Or is this enough of a one-off scenario that it's not worth the effort?

@wfurt
Copy link
Member

wfurt commented Feb 9, 2022

Is there any interest in generalized support for communicating a SkipTestException from a RemoteExecutor callback back to

I'm not sure @jkoritzinsky. That would be handy in cases when the skip is determined inside of the code that runs inside of the remote. It seems like in most cases we can determine this upfront. In this case, this is caused by this test calling another test that now has SkipTest and this dependency was not obvious.

@jkoritzinsky
Copy link
Member

@wfurt In that case, I think this is an obtuse-enough scenario that we don't need to add support to RemoteExecutor directly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EventSource_SuccessfulHandshake_LogsStartStop failures on Windows11
4 participants