Skip to content

Conversation

@TimothyMothra
Copy link

@TimothyMothra TimothyMothra commented Oct 9, 2019

SetsEndpointAddressPropertyToSpecifiedValue.

This test has failed 44 times in the last 30 days, but the code is literally verifying that the constructor sets a public property!!!!????

This test failed two days ago:
“Error calling Test Cleanup method for test class Microsoft.ApplicationInsights.Channel.TransmissionTest+Constructor: System.Net.Http.HttpRequestException: Error while copying content to a stream. ---> System.IO.IOException: Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host. ---> System.Net.Sockets.SocketException: An existing connection was forcibly closed by the remote host”

This is suspicious because this test doesn’t invoke any network operations.
Taking a closer look, this test class inherits some base classes. Reading the comments these seem to only exist to support testing Async operations in .Net 3.5 !!!!
Soooo….. I’m going to remove some of these complicated test dependencies.

Changes

  • I removed the dependency on the subclasses. This test is literally only testing the constructor. All this extra stuff is not needed.
  • I replaced the test Uri with localhost. The Uri class does some non-trivial validation so only creating one should reduce some of the work. Also, IF for some reason this tries to make an outbound test, it should only ping the local host and fail fast.
  • I removed the dependenc8y on the AssertEx utility class. This is unnecessary, the MSTest framework is more than capable of catching expected exceptions.

@TimothyMothra
Copy link
Author

Unrelated Test failed: ProactivelySampledInTelemetryCapturedWhenProactiveSamplingRateIsHigherThanTarget

@TimothyMothra TimothyMothra merged commit 12b0215 into develop Oct 9, 2019
@TimothyMothra TimothyMothra deleted the tilee/fix_TransmissionTests branch October 28, 2019 22:10
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.

3 participants