Skip to content

[release/6.0] Increase some of the DnsEndPointTest timeouts #58617

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

Merged
merged 7 commits into from
Sep 8, 2021

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Sep 3, 2021

Backport of #58129 to release/6.0

Long story short:
I have spent several hours trying to get a test run on SLES in order to repro #57929, no success so far, and it may take days to make progress, since I'm unfamiliar with SLES.

I recommend to bump the timeout values for now, and see if it helps with the issue. If not, we may invest into another round of investigation.

(hopefully) fixes #57929

/cc @karelz @antonfirsov

Customer Impact

Massive failures (6 per day) in CI mostly on SLES and only in release/6.0* branches.

Testing

Outerloop test run

Risk

Low: Test-only change - changing only timeouts.

@ghost ghost added the area-System.Net label Sep 3, 2021
@ghost
Copy link

ghost commented Sep 3, 2021

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

Issue Details

Backport of #58129 to release/6.0

/cc @karelz @antonfirsov

Customer Impact

Testing

Risk

Author: github-actions[bot]
Assignees: -
Labels:

area-System.Net

Milestone: -

@karelz
Copy link
Member

karelz commented Sep 3, 2021

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@karelz karelz added this to the 6.0.0 milestone Sep 3, 2021
@karelz
Copy link
Member

karelz commented Sep 6, 2021

Unrelated runtime-staging failures, rerunning them.

@karelz
Copy link
Member

karelz commented Sep 7, 2021

@danmoseley this is ready for your approval - test only change.

Comment on lines +11 to +12
public const int PassingTestTimeout = 10_000;
public const int PassingTestLongTimeout = 30_000;
Copy link
Member

Choose a reason for hiding this comment

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

Why not just have PassingTestTimeout be 30s rather than having two different timeouts? The purpose of the "passing" timeout is we expect these to always pass in well under this timeout, and if it times out, the test is going to fail. Taking an extra 20 seconds in the case of a failing test shouldn't be a problem, since tests shouldn't be failing.

Copy link
Member

Choose a reason for hiding this comment

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

It was introduced based on CR feedback in the PR on main: #58129 (comment)
@stephentoub I assume your comment is non-blocking for 6.0 merging. We can reopen the discussion in main once @antonfirsov is back.

@danmoseley danmoseley merged commit a92e3fd into release/6.0 Sep 8, 2021
@danmoseley danmoseley deleted the backport/pr-58129-to-release/6.0 branch September 8, 2021 01:49
@danmoseley
Copy link
Member

Approved - test only stability change. The tweak mentioned can happen in main.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 8, 2021
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.

4 participants