From da6c5fbf7ff589b73f8cb834925206b6464ef632 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 6 Jun 2012 19:05:51 -0400 Subject: [PATCH 1/5] MAINT-1144: Unblock build for previous commit. Today, 2012-06-06, LLHTTPClient::get("http://www.google.com/") has stopped working. This is unrelated to viewer code changes: it crosses repo boundaries. Skip the afflicted tests to get builds working until we can recast the tests to avoid the flaky dependency. --- indra/test/llhttpclient_tut.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/indra/test/llhttpclient_tut.cpp b/indra/test/llhttpclient_tut.cpp index 4b4046632c..1e0296918c 100644 --- a/indra/test/llhttpclient_tut.cpp +++ b/indra/test/llhttpclient_tut.cpp @@ -257,6 +257,7 @@ namespace tut template<> template<> void HTTPClientTestObject::test<1>() { + skip("google.com unit tests stopped working 2012-06-06"); LLHTTPClient::get("http://www.google.com/", newResult()); runThePump(); ensureStatusOK(); @@ -362,6 +363,7 @@ namespace tut template<> template<> void HTTPClientTestObject::test<8>() { + skip("google.com unit tests stopped working 2012-06-06"); // This is testing for the presence of the Header in the returned results // from an HTTP::get call. LLHTTPClient::get("http://www.google.com/", newResult()); @@ -373,6 +375,7 @@ namespace tut template<> template<> void HTTPClientTestObject::test<9>() { + skip("google.com unit tests stopped working 2012-06-06"); LLHTTPClient::head("http://www.google.com/", newResult()); runThePump(); ensureStatusOK(); From cd05a244da8d3249db1f3c1e8db215eb28da2816 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 6 Jun 2012 19:57:58 -0400 Subject: [PATCH 2/5] MAINT-1144: Break out llhttpclient_tut as standalone integration test. indra/test/llhttpclient_tut.cpp ==> indra/llmessage/tests/llhttpclient_test.cpp, with corresponding CMakeLists.txt changes in their respective directories. This first commit merely moves the test source file without changing it -- separating out moves from edits simplifies code review. --- indra/llmessage/CMakeLists.txt | 8 ++++++++ .../tests/llhttpclient_test.cpp} | 0 indra/test/CMakeLists.txt | 1 - 3 files changed, 8 insertions(+), 1 deletion(-) rename indra/{test/llhttpclient_tut.cpp => llmessage/tests/llhttpclient_test.cpp} (100%) diff --git a/indra/llmessage/CMakeLists.txt b/indra/llmessage/CMakeLists.txt index 0f40a670fa..d98781e9e6 100644 --- a/indra/llmessage/CMakeLists.txt +++ b/indra/llmessage/CMakeLists.txt @@ -254,6 +254,14 @@ if (LL_TESTS) "${CMAKE_CURRENT_SOURCE_DIR}/tests/test_llsdmessage_peer.py" ) + LL_ADD_INTEGRATION_TEST( + llhttpclient + "llhttpclient.cpp" + "${test_libs}" + ${PYTHON_EXECUTABLE} + "${CMAKE_CURRENT_SOURCE_DIR}/tests/test_llsdmessage_peer.py" + ) + LL_ADD_INTEGRATION_TEST(llavatarnamecache "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llhost "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llpartdata "" "${test_libs}") diff --git a/indra/test/llhttpclient_tut.cpp b/indra/llmessage/tests/llhttpclient_test.cpp similarity index 100% rename from indra/test/llhttpclient_tut.cpp rename to indra/llmessage/tests/llhttpclient_test.cpp diff --git a/indra/test/CMakeLists.txt b/indra/test/CMakeLists.txt index 328ab4ca51..816f1d7175 100644 --- a/indra/test/CMakeLists.txt +++ b/indra/test/CMakeLists.txt @@ -37,7 +37,6 @@ set(test_SOURCE_FILES lldoubledispatch_tut.cpp llevents_tut.cpp llhttpdate_tut.cpp - llhttpclient_tut.cpp llhttpnode_tut.cpp lliohttpserver_tut.cpp llmessageconfig_tut.cpp From 3d1cb7ef111cbd5724262077702e0dc2a3d6998d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 6 Jun 2012 23:12:11 -0400 Subject: [PATCH 3/5] MAINT-1144: Re-enable skipped LLHTTPClient tests with local server. Over the years we've skipped more and more of the tests in llhttpclient_test.cpp (nee llhttpclient_tut.cpp) because they've relied on particular behaviors from Internet sites not under our control. We skipped a test that fetches llsd+xml from secondlife.com because "secondlife.com is not reliable enough for unit tests." We skipped a test that tries to observe a failure with "http://www.invalid" because some local ISPs turn "no such domain" DNS errors into valid pages offering the requester to buy the specified domain name. Today we've had to skip tests attempting to contact "http://www.google.com" for reasons we haven't yet diagnosed, but that probably have to do with Google's IPv6 rollout. Use local temp server test_llsdmessage_peer.py as the success destination, eliminating DNS, Internet access and remote server behavior as failure modes. Use idle localhost port for failure test. Re-enable all skipped LLHTTPClient tests! Re-enable on Windows! In support of these tests, modify test_llsdmessage_peer.py: Support HEAD as no-data variant of GET. Change GET result dict to avoid resembling an error response -- confusing. Make GET/POST return actual dict rather than undecorated string "success". Because of that last, change llcapabilitylistener_test.cpp and llsdmessage_test.cpp to extract "reply" key from response rather than expecting response to be a string itself. --- indra/llmessage/tests/llhttpclient_test.cpp | 40 +++++++------------ indra/llmessage/tests/llsdmessage_test.cpp | 2 +- .../llmessage/tests/test_llsdmessage_peer.py | 19 ++++++--- .../tests/llcapabilitylistener_test.cpp | 2 +- 4 files changed, 30 insertions(+), 33 deletions(-) diff --git a/indra/llmessage/tests/llhttpclient_test.cpp b/indra/llmessage/tests/llhttpclient_test.cpp index 1e0296918c..986fd5f3fc 100644 --- a/indra/llmessage/tests/llhttpclient_test.cpp +++ b/indra/llmessage/tests/llhttpclient_test.cpp @@ -1,5 +1,5 @@ /** - * @file llhttpclient_tut.cpp + * @file llhttpclient_test.cpp * @brief Testing the HTTP client classes. * * $LicenseInfo:firstyear=2006&license=viewerlgpl$ @@ -33,9 +33,6 @@ #include #include "linden_common.h" -// These are too slow on Windows to actually include in the build. JC -#if !LL_WINDOWS - #include "lltut.h" #include "llhttpclient.h" #include "llformat.h" @@ -46,6 +43,7 @@ #include "llsdhttpserver.h" #include "lliohttpserver.h" #include "lliosocket.h" +#include "stringize.h" namespace tut { @@ -83,7 +81,8 @@ namespace tut struct HTTPClientTestData { public: - HTTPClientTestData() + HTTPClientTestData(): + local_server(STRINGIZE("http://127.0.0.1:" << getenv("PORT") << "/")) { apr_pool_create(&mPool, NULL); LLCurl::initClass(false); @@ -134,13 +133,14 @@ namespace tut delete mServerPump; mServerPump = NULL; } - + + const std::string local_server; + private: apr_pool_t* mPool; LLPumpIO* mServerPump; LLPumpIO* mClientPump; - protected: void ensureStatusOK() { @@ -257,8 +257,7 @@ namespace tut template<> template<> void HTTPClientTestObject::test<1>() { - skip("google.com unit tests stopped working 2012-06-06"); - LLHTTPClient::get("http://www.google.com/", newResult()); + LLHTTPClient::get(local_server, newResult()); runThePump(); ensureStatusOK(); ensure("result object wasn't destroyed", mResultDeleted); @@ -267,8 +266,8 @@ namespace tut template<> template<> void HTTPClientTestObject::test<2>() { - skip("error test depends on dev's local ISP not supplying \"helpful\" search page"); - LLHTTPClient::get("http://www.invalid", newResult()); + // Please nobody listen on this particular port... + LLHTTPClient::get("http://127.0.0.1:7950", newResult()); runThePump(); ensureStatusError(); } @@ -345,28 +344,22 @@ namespace tut // won't ever let it run. Instead get from a known LLSD // source and compare results with the non-blocking get which // is tested against the mini server earlier. - skip("secondlife.com is not reliable enough for unit tests."); - - - LLSD expected; - - LLHTTPClient::get("http://secondlife.com/xmlhttp/homepage.php", newResult()); + LLHTTPClient::get(local_server, newResult()); runThePump(); ensureStatusOK(); - expected = getResult(); + LLSD expected = getResult(); LLSD result; - result = LLHTTPClient::blockingGet("http://secondlife.com/xmlhttp/homepage.php"); + result = LLHTTPClient::blockingGet(local_server); LLSD body = result["body"]; ensure_equals("echoed result matches", body.size(), expected.size()); } template<> template<> void HTTPClientTestObject::test<8>() { - skip("google.com unit tests stopped working 2012-06-06"); // This is testing for the presence of the Header in the returned results // from an HTTP::get call. - LLHTTPClient::get("http://www.google.com/", newResult()); + LLHTTPClient::get(local_server, newResult()); runThePump(); ensureStatusOK(); LLSD header = getHeader(); @@ -375,12 +368,9 @@ namespace tut template<> template<> void HTTPClientTestObject::test<9>() { - skip("google.com unit tests stopped working 2012-06-06"); - LLHTTPClient::head("http://www.google.com/", newResult()); + LLHTTPClient::head(local_server, newResult()); runThePump(); ensureStatusOK(); ensure("result object wasn't destroyed", mResultDeleted); } } - -#endif // !LL_WINDOWS diff --git a/indra/llmessage/tests/llsdmessage_test.cpp b/indra/llmessage/tests/llsdmessage_test.cpp index 0f2c069303..31a791e4b4 100644 --- a/indra/llmessage/tests/llsdmessage_test.cpp +++ b/indra/llmessage/tests/llsdmessage_test.cpp @@ -143,7 +143,7 @@ namespace tut httpPump.post(request); ensure("got response", netio.pump()); ensure("success response", success); - ensure_equals(result.asString(), "success"); + ensure_equals(result["reply"].asString(), "success"); body["status"] = 499; body["reason"] = "custom error message"; diff --git a/indra/llmessage/tests/test_llsdmessage_peer.py b/indra/llmessage/tests/test_llsdmessage_peer.py index 22edd9dad8..fe4f3a8c01 100644 --- a/indra/llmessage/tests/test_llsdmessage_peer.py +++ b/indra/llmessage/tests/test_llsdmessage_peer.py @@ -78,25 +78,32 @@ class TestHTTPRequestHandler(BaseHTTPRequestHandler): ## debug("root node tag %s", tree.getroot().tag) ## return llsd.to_python(tree.getroot()) - def do_GET(self): + def do_HEAD(self): + self.do_GET(withdata=False) + + def do_GET(self, withdata=True): # Of course, don't attempt to read data. - self.answer(dict(reply="success", status=500, - reason="Your GET operation requested failure")) + data = dict(reply="success", body="avatar", random=17) + self.answer(data, withdata=withdata) def do_POST(self): # Read the provided POST data. self.answer(self.read_xml()) - def answer(self, data): + def answer(self, data, withdata=True): debug("%s.answer(%s): self.path = %r", self.__class__.__name__, data, self.path) if "fail" not in self.path: - response = llsd.format_xml(data.get("reply", llsd.LLSD("success"))) + data = data.copy() # we're going to modify + # Ensure there's a "reply" key in data, even if there wasn't before + data["reply"] = data.get("reply", llsd.LLSD("success")) + response = llsd.format_xml(data) debug("success: %s", response) self.send_response(200) self.send_header("Content-type", "application/llsd+xml") self.send_header("Content-Length", str(len(response))) self.end_headers() - self.wfile.write(response) + if withdata: + self.wfile.write(response) else: # fail requested status = data.get("status", 500) # self.responses maps an int status to a (short, long) pair of diff --git a/indra/newview/tests/llcapabilitylistener_test.cpp b/indra/newview/tests/llcapabilitylistener_test.cpp index 2ad08dc1f3..e1cbd28f92 100644 --- a/indra/newview/tests/llcapabilitylistener_test.cpp +++ b/indra/newview/tests/llcapabilitylistener_test.cpp @@ -156,7 +156,7 @@ namespace tut regionPump.post(request); ensure("got response", netio.pump()); ensure("success response", success); - ensure_equals(result.asString(), "success"); + ensure_equals(result["reply"].asString(), "success"); body["status"] = 499; body["reason"] = "custom error message"; From d167ebe35f8cdec1ca88e0d817e2878f14a5aa68 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Thu, 7 Jun 2012 05:46:47 -0400 Subject: [PATCH 4/5] MAINT-1144: Try to fix Windows build error in llhttpclient_test.cpp. --- indra/llmessage/tests/llhttpclient_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/indra/llmessage/tests/llhttpclient_test.cpp b/indra/llmessage/tests/llhttpclient_test.cpp index 986fd5f3fc..843c3bcc4b 100644 --- a/indra/llmessage/tests/llhttpclient_test.cpp +++ b/indra/llmessage/tests/llhttpclient_test.cpp @@ -363,7 +363,7 @@ namespace tut runThePump(); ensureStatusOK(); LLSD header = getHeader(); - ensure_equals("got a header", header.emptyMap().asBoolean(), FALSE); + ensure("got a header", ! header.emptyMap().asBoolean()); } template<> template<> void HTTPClientTestObject::test<9>() From 14335ff562825a04c5ada50bb80d03c2dfa21cad Mon Sep 17 00:00:00 2001 From: Oz Linden Date: Thu, 7 Jun 2012 13:39:23 -0400 Subject: [PATCH 5/5] tag merge of DRTVWR-162 --- .hgtags | 1 + 1 file changed, 1 insertion(+) diff --git a/.hgtags b/.hgtags index 4f3f55ad94..839a34b34d 100644 --- a/.hgtags +++ b/.hgtags @@ -297,3 +297,4 @@ d29a260119f8d5a5d168e25fed0c7ea6b3f40161 3.3.2-beta1 c623bbc854b6f7ee1b33a3718f76715046aa2937 viewer-release-candidate 675668bd24d3bea570814f71762a2a806f7e1b8d viewer-release-candidate 675668bd24d3bea570814f71762a2a806f7e1b8d 3.3.2-release +600f3b3920d94de805ac6dc8bb6def9c069dd360 DRTVWR-162