-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[release/5.0] Fix OS version check and testhost copying in stress tests #41321
Conversation
Tagging subscribers to this area: @safern, @ViktorHofer |
Tagging subscribers to this area: @dotnet/ncl |
/azp run runtime-libraries stress-http |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries stress-ssl |
Azure Pipelines successfully started running 1 pipeline(s). |
@karelz We need to port it back to 5.0 to investigate stress test failures. |
We found an issue in the master branch. Holding off the backport for now. |
Fixed docker files not using currently built runtime for http stress test.
/azp run runtime-libraries stress-http |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-libraries stress-ssl |
Azure Pipelines successfully started running 1 pipeline(s). |
Linux: Based on modified date, the tests are running on freshly built bits. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with nits
public bool Equals([AllowNull] T left, [AllowNull] T right) => left != null && left.Equals(right, StructuralComparisons.StructuralEqualityComparer); | ||
public int GetHashCode([DisallowNull] T value) => value.GetHashCode(StructuralComparisons.StructuralEqualityComparer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public bool Equals([AllowNull] T left, [AllowNull] T right) => left != null && left.Equals(right, StructuralComparisons.StructuralEqualityComparer); | |
public int GetHashCode([DisallowNull] T value) => value.GetHashCode(StructuralComparisons.StructuralEqualityComparer); | |
public bool Equals(T? left, T? right) => left != null && left.Equals(right, StructuralComparisons.StructuralEqualityComparer); | |
public int GetHashCode(T value) => value.GetHashCode(StructuralComparisons.StructuralEqualityComparer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in #41513, it's not necessary to be ported. The code works as-is.
public bool Equals([AllowNull] T left, [AllowNull] T right) => left != null && left.Equals(right, StructuralComparisons.StructuralEqualityComparer); | ||
public int GetHashCode([DisallowNull] T value) => value.GetHashCode(StructuralComparisons.StructuralEqualityComparer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public bool Equals([AllowNull] T left, [AllowNull] T right) => left != null && left.Equals(right, StructuralComparisons.StructuralEqualityComparer); | |
public int GetHashCode([DisallowNull] T value) => value.GetHashCode(StructuralComparisons.StructuralEqualityComparer); | |
public bool Equals(T? left, T? right) => left != null && left.Equals(right, StructuralComparisons.StructuralEqualityComparer); | |
public int GetHashCode(T value) => value.GetHashCode(StructuralComparisons.StructuralEqualityComparer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in #41513, it's not necessary to be ported. The code works as-is.
CI runtime failure |
The failures from http-stress test are being tracked in #40388. They're not the goal of this PR, this PR is just fixing the infra running the tests. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
@ericstj okay to merge this? no changes to product code, only to our test suite so we can run stress tests for 5.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved. Test only
Make sure it’s green or failures are understood and issues filed |
1 similar comment
Make sure it’s green or failures are understood and issues filed |
CI runtime failures: |
Backport of #40980 and #41397 to release/5.0
/cc @alnikola
Customer Impact
This PR fixes broken HTTP and SSL stress pipelines. No direct customer impact, but having the stress pipeline up and running is crucial to unblock investigation of stress failures we observed on the master branch.
Testing
Risk
None. No changes to production code.