Skip to content

Improve HttpClient and WebClient sample to test all public API methods#830

Merged
zacharycmontoya merged 12 commits into
masterfrom
zach/tests/http-clients
Aug 7, 2020
Merged

Improve HttpClient and WebClient sample to test all public API methods#830
zacharycmontoya merged 12 commits into
masterfrom
zach/tests/http-clients

Conversation

@zacharycmontoya

@zacharycmontoya zacharycmontoya commented Jul 31, 2020

Copy link
Copy Markdown
Contributor

Changes proposed in this pull request:

  • Remove WebRequest-related tests out of the all-encompassing Samples.HttpMessageHandler sample application and place it into a new sample application Samples.WebRequest
  • Rename test files so they either say "HttpMessageHandler" or "WebRequest", which matches the integration name
  • Improve Samples.HttpMessageHandler sample application by executing all xxxAsync methods in the HttpClient public API
  • Improve Samples.WebRequest sample application by executing all Downloadxxx and Uploadxxx methods in the WebClient public API and executing GetResponse and GetResponseAsync methods in the WebRequest public API

@DataDog/apm-dotnet

@zacharycmontoya zacharycmontoya added area:tests unit tests, integration tests area:test-apps apps used to test integrations labels Jul 31, 2020
@zacharycmontoya
zacharycmontoya requested a review from a team as a code owner July 31, 2020 20:51
@zacharycmontoya zacharycmontoya self-assigned this Jul 31, 2020

@lucaspimentel lucaspimentel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The methods we instrument in WebRequest ( GetResponse() and GetResponseAsync() ) are public APIs. Are we sure the new tests call both of those methods? Should we have tests that call them directly?

@zacharycmontoya

Copy link
Copy Markdown
Contributor Author

The methods we instrument in WebRequest ( GetResponse() and GetResponseAsync() ) are public APIs. Are we sure the new tests call both of those methods? Should we have tests that call them directly?

Good idea. I've implemented that in commit 1e0df4b

@zacharycmontoya
zacharycmontoya force-pushed the zach/tests/http-clients branch from bb56a9d to 60a83a5 Compare August 5, 2020 00:46
@zacharycmontoya
zacharycmontoya merged commit f57a8c6 into master Aug 7, 2020
@zacharycmontoya
zacharycmontoya deleted the zach/tests/http-clients branch August 7, 2020 23:06
@zacharycmontoya zacharycmontoya added this to the 1.19.2 milestone Aug 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:test-apps apps used to test integrations area:tests unit tests, integration tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants