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

Use of www.google.com in tests #77

Closed
NiteshKant opened this issue Mar 25, 2014 · 4 comments
Closed

Use of www.google.com in tests #77

NiteshKant opened this issue Mar 25, 2014 · 4 comments

Comments

@NiteshKant
Copy link
Member

A lot of tests in HttpClientTest tries to connect to "www.google.com".
This can easily be replaced by starting a local RxServer on a port. The dependency on a website can cause flakiness of tests and also adds an unnecessary requirement of connecting to the internet for running the tests (Yes I want to run these tests on a flight :) )

@allenxwang
Copy link

Sure, just get wifi on the flight :))

The reason to pick google.com is that it is a reliable site that returns 200 response code and keeps the connection alive. Another reason is that it sends a large payload in multiple chunks so we actually test that the aggregator works.

As long as we can satisfy the above, I am fine with replacing it with an RxServer.

On the other hand, I feel that not always relying on RxServer gives us more confidence that our client is compatible with other HTTP server that conforms to the standard. Same applies to RxServer. If we can have test that uses non RxClient to communicate to it, more compatible our implementation will be.

@NiteshKant
Copy link
Member Author

The reason to pick google.com is that it is a reliable site that returns 200 response code and keeps the connection alive. Another reason is that it sends a large payload in multiple chunks so we actually test that the aggregator works.

So, if tomorrow, for whatever reason google.com starts sending a more lighter response or stops keeping the connection alive or gives a redirect to lets says google.co.in when queried from India, we stop testing what we believe we are testing? Worst still, In the extreme case of a google.com outage (in the light of the recent instance), our test start failing?
One of the basics of unit testing is to have a predictable, consistent test case. The established way of doing so when an external interaction is required is a mock/fake. Using RxServer is achieving that mock/fake functionality. The test class in question is already using RxServer for most of the other tests. I think all the requirements you have listed is already being handled by the existing mock. Any reason why we can not use that?
Having said that, if you are more confident in using some other mocking library, that is fine too.

On the other hand, I feel that not always relying on RxServer gives us more confidence that our client is compatible with other HTTP server that conforms to the standard. Same applies to RxServer. If we can have test that uses non RxClient to communicate to it, more compatible our implementation will be.

I disagree. If we want confidence in being compatible with the HTTP spec, we should write test cases for that conformance, rather than assuming that an external server/client is adhering to the spec. By just using an external client/server we can in no way be confident that we are adhering to the specs.

BTW, I will pass on the bill to you for the WIFI in the flight ;)

@benjchristensen
Copy link
Member

It's good to move to internal implementations ... I used google.com in very early things.

We can and should simulate all server and client things we want by building clients and servers that do them.

@NiteshKant
Copy link
Member Author

All usages of www.google.com have been removed apart from the test HttpClientTest. testConnectException2() which is simulating connect timeout. There isn't a way to easily simulate it and hence I have left it like that.

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

No branches or pull requests

3 participants