You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2019/11/12 22:33:58 UTC
[impala] branch master updated: IMPALA-9082: make WebserverTest
error checking stricter
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new a4743fd IMPALA-9082: make WebserverTest error checking stricter
a4743fd is described below
commit a4743fd9bb9158ac5c4de55f77276f39b4570346
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
AuthorDate: Tue Oct 22 14:40:08 2019 -0700
IMPALA-9082: make WebserverTest error checking stricter
WebserverTest::TestWithSpnego has been flaky lately, but I have been
unable to repro it. This patch is an attempt to make it easier to
debug the issue the next time it shows up in automated builds.
The test performs a GET that is expected to fail and then checks that
the metrics show the failed GET. The flaky failures occur when the
metrics do not show a failed attempt.
It appears that what's happening is the GET is failing before actually
reaching the webserver. However, because the GET is expected to fail
and because we only verify that it did fail by checking that HttpGet()
returned some error status, whatever unexpected error is occuring is
getting lost.
This patch instead checks the actual text of the error that is
returned by HttpGet() to make sure it is correct and logs the error if
it isn't.
Change-Id: I820336271cf25130538ceae2eed10a72a73d2adc
Reviewed-on: http://gerrit.cloudera.org:8080/14672
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/testutil/gtest-util.h | 9 +++++++++
be/src/util/webserver-test.cc | 6 ++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/be/src/testutil/gtest-util.h b/be/src/testutil/gtest-util.h
index 68281b7..88fa7d3 100644
--- a/be/src/testutil/gtest-util.h
+++ b/be/src/testutil/gtest-util.h
@@ -41,12 +41,21 @@ namespace impala {
#define EXPECT_STR_CONTAINS(str, substr) \
EXPECT_PRED_FORMAT2(testing::IsSubstring, substr, str)
+// Error code matches.
#define EXPECT_ERROR(status, err) \
do { \
const Status& status_ = (status); \
EXPECT_EQ(status_.code(), err) << "Error: " << status_.GetDetail(); \
} while (0)
+// Text of error message matches.
+#define ASSERT_ERROR_MSG(status, err) \
+ do { \
+ const Status& status_ = (status); \
+ ASSERT_FALSE(status_.ok()); \
+ ASSERT_EQ(status_.msg().msg(), err) << "Error: " << status_.msg().msg(); \
+ } while (0)
+
// Basic main() function to be used in gtest unit tests. Does not start a JVM and does
// not initialize the FE.
#define IMPALA_TEST_MAIN() \
diff --git a/be/src/util/webserver-test.cc b/be/src/util/webserver-test.cc
index fcab593..d13c524 100644
--- a/be/src/util/webserver-test.cc
+++ b/be/src/util/webserver-test.cc
@@ -383,7 +383,8 @@ TEST(Webserver, TestWithSpnego) {
// Don't expect HTTP requests to work without Kerberos credentials.
stringstream contents;
- ASSERT_FALSE(HttpGet("localhost", FLAGS_webserver_port, "/", &contents).ok());
+ ASSERT_ERROR_MSG(HttpGet("localhost", FLAGS_webserver_port, "/", &contents),
+ "Unexpected status code: 401");
// There should be one failed auth attempt.
CheckAuthMetrics(&metrics, 0, 1, 0, 0);
@@ -445,7 +446,8 @@ TEST(Webserver, StartWithPasswordFileTest) {
// Don't expect HTTP requests to work without a password
stringstream contents;
- ASSERT_FALSE(HttpGet("localhost", FLAGS_webserver_port, "/", &contents).ok());
+ ASSERT_ERROR_MSG(HttpGet("localhost", FLAGS_webserver_port, "/", &contents),
+ "Unexpected status code: 401");
}
TEST(Webserver, StartWithMissingPasswordFileTest) {