Merge pull request #1 from grpc/master

Update fork

.github/ISSUE_TEMPLATE/bug_report.md

@@ -2,7 +2,7 @@
 name: Report a bug
 about: Create a report to help us improve
 labels: kind/bug, priority/P2
-assignees: nicolasnoble
+assignees: yashykt
 
 ---
 

.github/ISSUE_TEMPLATE/cleanup_request.md

@@ -2,7 +2,7 @@
 name: Request a cleanup
 about: Suggest a cleanup in our repository
 labels: kind/internal cleanup, priority/P2
-assignees: nicolasnoble
+assignees: yashykt
 
 ---
 

.github/ISSUE_TEMPLATE/feature_request.md

@@ -2,7 +2,7 @@
 name: Request a feature
 about: Suggest an idea for this project
 labels: kind/enhancement, priority/P2
-assignees: nicolasnoble
+assignees: yashykt
 
 ---
 

.github/ISSUE_TEMPLATE/question.md

@@ -2,7 +2,7 @@
 name: Ask a question
 about: Ask a question
 labels: kind/question, priority/P3
-assignees: nicolasnoble
+assignees: yashykt
 
 ---
 

.github/pull_request_template.md

@@ -8,4 +8,4 @@ If you know who should review your pull request, please remove the mentioning be
 
 -->
 
-@nicolasnoble
+@yashykt

Makefile

@@ -2196,6 +2196,8 @@ test_cxx: buildtests_cxx
 	$(Q) $(BINDIR)/$(CONFIG)/channelz_registry_test || ( echo test channelz_registry_test failed ; exit 1 )
 	$(E) "[RUN]     Testing channelz_service_test"
 	$(Q) $(BINDIR)/$(CONFIG)/channelz_service_test || ( echo test channelz_service_test failed ; exit 1 )
+	$(E) "[RUN]     Testing channelz_test"
+	$(Q) $(BINDIR)/$(CONFIG)/channelz_test || ( echo test channelz_test failed ; exit 1 )
 	$(E) "[RUN]     Testing cli_call_test"
 	$(Q) $(BINDIR)/$(CONFIG)/cli_call_test || ( echo test cli_call_test failed ; exit 1 )
 	$(E) "[RUN]     Testing client_callback_end2end_test"

build_autogenerated.yaml

@@ -5466,7 +5466,6 @@ targets:
 - name: channelz_test
   gtest: true
   build: test
-  run: false
   language: c++
   headers:
   - test/cpp/util/channel_trace_proto_helper.h

doc/PROTOCOL-HTTP2.md

@@ -52,12 +52,12 @@ Request-Headers are delivered as HTTP2 headers in HEADERS + CONTINUATION frames.
 
 HTTP2 requires that reserved headers, ones starting with ":" appear before all other headers. Additionally implementations should send **Timeout** immediately after the reserved headers and they should send the **Call-Definition** headers before sending **Custom-Metadata**.
 
-Some gRPC implementations may allow the **Path** format shown above
-to be overridden, but this functionality is strongly discouraged.
-gRPC does not go out of its way to break users that are using this kind
-of override, but we do not actively support it, and some functionality
-(e.g., service config support) will not work when the path is not of
-the form shown above.
+**Path** is case-sensitive. Some gRPC implementations may allow the **Path**
+format shown above to be overridden, but this functionality is strongly
+discouraged. gRPC does not go out of its way to break users that are using this
+kind of override, but we do not actively support it, and some functionality
+(e.g., service config support) will not work when the path is not of the form
+shown above.
 
 If **Timeout** is omitted a server should assume an infinite timeout. Client implementations are free to send a default minimum timeout based on their deployment requirements.
 

src/php/tests/interop/interop_client.php

@@ -116,6 +116,53 @@ function performLargeUnary($stub, $fillUsername = false,
     return $result;
 }
 
+/**
+ * Run the client_compressed_unary test.
+ *
+ * @param $stub Stub object that has service methods
+ */
+function clientCompressedUnary($stub)
+{
+    $request_len = 271828;
+    $response_len = 314159;
+    $falseBoolValue = new Grpc\Testing\BoolValue(['value' => false]);
+    $trueBoolValue = new Grpc\Testing\BoolValue(['value' => true]);
+    // 1. Probing for compression-checks support
+    $payload = new Grpc\Testing\Payload([
+        'body' => str_repeat("\0", $request_len),
+    ]);
+    $request = new Grpc\Testing\SimpleRequest([
+        'payload' => $payload,
+        'response_size' => $response_len,
+        'expect_compressed' => $trueBoolValue, // lie
+    ]);
+    list($result, $status) = $stub->UnaryCall($request, [], [])->wait();
+    hardAssert(
+        $status->code === GRPC\STATUS_INVALID_ARGUMENT,
+        'Received unexpected UnaryCall status code: ' .
+            $status->code
+    );
+    // 2. with/without compressed message
+    foreach ([true, false] as $compression) {
+        $request->setExpectCompressed($compression ? $trueBoolValue : $falseBoolValue);
+        $metadata = $compression ? [
+            'grpc-internal-encoding-request' => ['gzip'],
+        ] : [];
+        list($result, $status) = $stub->UnaryCall($request, $metadata, [])->wait();
+        hardAssertIfStatusOk($status);
+        hardAssert($result !== null, 'Call returned a null response');
+        $payload = $result->getPayload();
+        hardAssert(
+            strlen($payload->getBody()) === $response_len,
+            'Payload had the wrong length'
+        );
+        hardAssert(
+            $payload->getBody() === str_repeat("\0", $response_len),
+            'Payload had the wrong content'
+        );
+    }
+}
+
 /**
  * Run the service account credentials auth test.
  *
@@ -256,6 +303,68 @@ function clientStreaming($stub)
                'aggregated_payload_size was incorrect');
 }
 
+/**
+ * Run the client_compressed_streaming test.
+ *
+ * @param $stub Stub object that has service methods
+ */
+function clientCompressedStreaming($stub)
+{
+    $request_len = 27182;
+    $request2_len = 45904;
+    $response_len = 73086;
+    $falseBoolValue = new Grpc\Testing\BoolValue(['value' => false]);
+    $trueBoolValue = new Grpc\Testing\BoolValue(['value' => true]);
+
+    // 1. Probing for compression-checks support
+
+    $payload = new Grpc\Testing\Payload([
+        'body' => str_repeat("\0", $request_len),
+    ]);
+    $request = new Grpc\Testing\StreamingInputCallRequest([
+        'payload' => $payload,
+        'expect_compressed' => $trueBoolValue, // lie
+    ]);
+
+    $call = $stub->StreamingInputCall();
+    $call->write($request);
+    list($result, $status) = $call->wait();
+    hardAssert(
+        $status->code === GRPC\STATUS_INVALID_ARGUMENT,
+        'Received unexpected StreamingInputCall status code: ' .
+            $status->code
+    );
+
+    // 2. write compressed message
+
+    $call = $stub->StreamingInputCall([
+        'grpc-internal-encoding-request' => ['gzip'],
+    ]);
+    $request->setExpectCompressed($trueBoolValue);
+    $call->write($request);
+
+    // 3. write uncompressed message
+
+    $payload2 = new Grpc\Testing\Payload([
+        'body' => str_repeat("\0", $request2_len),
+    ]);
+    $request->setPayload($payload2);
+    $request->setExpectCompressed($falseBoolValue);
+    $call->write($request, [
+        'flags' => 0x02 // GRPC_WRITE_NO_COMPRESS
+    ]);
+
+    // 4. verify response
+
+    list($result, $status) = $call->wait();
+
+    hardAssertIfStatusOk($status);
+    hardAssert(
+        $result->getAggregatedPayloadSize() === $response_len,
+        'aggregated_payload_size was incorrect'
+    );
+}
+
 /**
  * Run the server_streaming test.
  *
@@ -673,6 +782,12 @@ function interop_main($args, $stub = false)
         case 'per_rpc_creds':
             perRpcCreds($stub, $args);
             break;
+        case 'client_compressed_unary':
+            clientCompressedUnary($stub);
+            break;
+        case 'client_compressed_streaming':
+            clientCompressedStreaming($stub);
+            break;
         default:
             echo "Unsupported test case $test_case\n";
             exit(1);

test/core/channel/BUILD

@@ -87,7 +87,6 @@ grpc_cc_test(
     external_deps = [
         "gtest",
     ],
-    flaky = True,  # TODO(b/151792070)
     language = "C++",
     deps = [
         "//:gpr",

test/core/surface/sequential_connectivity_test.cc

@@ -16,8 +16,11 @@
  *
  */
 
+#include <vector>
+
 #include <grpc/grpc.h>
 #include <grpc/grpc_security.h>
+#include <grpc/impl/codegen/grpc_types.h>
 #include <grpc/support/alloc.h>
 #include <grpc/support/log.h>
 
@@ -36,16 +39,12 @@
 typedef struct test_fixture {
   const char* name;
   void (*add_server_port)(grpc_server* server, const char* addr);
-  grpc_channel* (*create_channel)(const char* addr);
+  // Have the creds here so all the channels will share the same one to enabled
+  // subchannel sharing if needed.
+  grpc_channel_credentials* creds;
 } test_fixture;
 
-/* TODO(yashykt): When our macos testing infrastructure becomes good enough, we
- * wouldn't need to reduce the number of connections on MacOS */
-#ifdef __APPLE__
 #define NUM_CONNECTIONS 100
-#else
-#define NUM_CONNECTIONS 1000
-#endif /* __APPLE__ */
 
 typedef struct {
   grpc_server* server;
@@ -61,10 +60,31 @@ static void server_thread_func(void* args) {
   GPR_ASSERT(ev.success == true);
 }
 
-static void run_test(const test_fixture* fixture) {
-  gpr_log(GPR_INFO, "TEST: %s", fixture->name);
+static grpc_channel* create_test_channel(const char* addr,
+                                         grpc_channel_credentials* creds,
+                                         bool share_subchannel) {
+  grpc_channel* channel = nullptr;
+  std::vector<grpc_arg> args;
+  args.push_back(grpc_channel_arg_integer_create(
+      const_cast<char*>(GRPC_ARG_USE_LOCAL_SUBCHANNEL_POOL),
+      !share_subchannel));
+  if (creds != nullptr) {
+    args.push_back(grpc_channel_arg_string_create(
+        const_cast<char*>(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG),
+        const_cast<char*>("foo.test.google.fr")));
+  }
+  grpc_channel_args channel_args = {args.size(), args.data()};
+  if (creds != nullptr) {
+    channel = grpc_secure_channel_create(creds, addr, &channel_args, nullptr);
+  } else {
+    channel = grpc_insecure_channel_create(addr, &channel_args, nullptr);
+  }
+  return channel;
+}
 
-  grpc_init();
+static void run_test(const test_fixture* fixture, bool share_subchannel) {
+  gpr_log(GPR_INFO, "TEST: %s sharing subchannel: %d", fixture->name,
+          share_subchannel);
 
   std::string addr =
       grpc_core::JoinHostPort("localhost", grpc_pick_unused_port_or_die());
@@ -83,7 +103,8 @@ static void run_test(const test_fixture* fixture) {
   grpc_completion_queue* cq = grpc_completion_queue_create_for_next(nullptr);
   grpc_channel* channels[NUM_CONNECTIONS];
   for (size_t i = 0; i < NUM_CONNECTIONS; i++) {
-    channels[i] = fixture->create_channel(addr.c_str());
+    channels[i] =
+        create_test_channel(addr.c_str(), fixture->creds, share_subchannel);
 
     gpr_timespec connect_deadline = grpc_timeout_seconds_to_deadline(30);
     grpc_connectivity_state state;
@@ -124,24 +145,12 @@ static void run_test(const test_fixture* fixture) {
   grpc_server_destroy(server);
   grpc_completion_queue_destroy(server_cq);
   grpc_completion_queue_destroy(cq);
-
-  grpc_shutdown();
 }
 
 static void insecure_test_add_port(grpc_server* server, const char* addr) {
   grpc_server_add_insecure_http2_port(server, addr);
 }
 
-static grpc_channel* insecure_test_create_channel(const char* addr) {
-  return grpc_insecure_channel_create(addr, nullptr, nullptr);
-}
-
-static const test_fixture insecure_test = {
-    "insecure",
-    insecure_test_add_port,
-    insecure_test_create_channel,
-};
-
 static void secure_test_add_port(grpc_server* server, const char* addr) {
   grpc_slice cert_slice, key_slice;
   GPR_ASSERT(GRPC_LOG_IF_ERROR(
@@ -161,7 +170,18 @@ static void secure_test_add_port(grpc_server* server, const char* addr) {
   grpc_server_credentials_release(ssl_creds);
 }
 
-static grpc_channel* secure_test_create_channel(const char* addr) {
+int main(int argc, char** argv) {
+  grpc::testing::TestEnvironment env(argc, argv);
+  grpc_init();
+
+  const test_fixture insecure_test = {
+      "insecure",
+      insecure_test_add_port,
+      nullptr,
+  };
+  run_test(&insecure_test, /*share_subchannel=*/true);
+  run_test(&insecure_test, /*share_subchannel=*/false);
+
   grpc_slice ca_slice;
   GPR_ASSERT(GRPC_LOG_IF_ERROR("load_file",
                                grpc_load_file(CA_CERT_PATH, 1, &ca_slice)));
@@ -170,31 +190,14 @@ static grpc_channel* secure_test_create_channel(const char* addr) {
   grpc_channel_credentials* ssl_creds =
       grpc_ssl_credentials_create(test_root_cert, nullptr, nullptr, nullptr);
   grpc_slice_unref(ca_slice);
-  grpc_arg ssl_name_override = {
-      GRPC_ARG_STRING,
-      const_cast<char*>(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG),
-      {const_cast<char*>("foo.test.google.fr")}};
-  grpc_channel_args* new_client_args =
-      grpc_channel_args_copy_and_add(nullptr, &ssl_name_override, 1);
-  grpc_channel* channel =
-      grpc_secure_channel_create(ssl_creds, addr, new_client_args, nullptr);
-  {
-    grpc_core::ExecCtx exec_ctx;
-    grpc_channel_args_destroy(new_client_args);
-  }
+  const test_fixture secure_test = {
+      "secure",
+      secure_test_add_port,
+      ssl_creds,
+  };
+  run_test(&secure_test, /*share_subchannel=*/true);
+  run_test(&secure_test, /*share_subchannel=*/false);
   grpc_channel_credentials_release(ssl_creds);
-  return channel;
-}
-
-static const test_fixture secure_test = {
-    "secure",
-    secure_test_add_port,
-    secure_test_create_channel,
-};
-
-int main(int argc, char** argv) {
-  grpc::testing::TestEnvironment env(argc, argv);
 
-  run_test(&insecure_test);
-  run_test(&secure_test);
+  grpc_shutdown();
 }

test/cpp/microbenchmarks/BUILD

@@ -282,7 +282,6 @@ grpc_cc_test(
 grpc_cc_test(
     name = "bm_opencensus_plugin",
     srcs = ["bm_opencensus_plugin.cc"],
-    flaky = True,  # TODO(b/151696309)
     language = "C++",
     deps = [
         ":helpers_secure",

tools/run_tests/generated/tests.json

@@ -4049,6 +4049,30 @@
     ], 
     "uses_polling": true
   }, 
+  {
+    "args": [], 
+    "benchmark": false, 
+    "ci_platforms": [
+      "linux", 
+      "mac", 
+      "posix", 
+      "windows"
+    ], 
+    "cpu_cost": 1.0, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "gtest": true, 
+    "language": "c++", 
+    "name": "channelz_test", 
+    "platforms": [
+      "linux", 
+      "mac", 
+      "posix", 
+      "windows"
+    ], 
+    "uses_polling": true
+  }, 
   {
     "args": [], 
     "benchmark": false, 

tools/run_tests/run_interop_tests.py

@@ -499,7 +499,7 @@ class PHPLanguage:
         return {}
 
     def unimplemented_test_cases(self):
-        return _SKIP_COMPRESSION + \
+        return _SKIP_SERVER_COMPRESSION + \
             _SKIP_DATA_FRAME_PADDING + \
             _SKIP_SPECIAL_STATUS_MESSAGE + \
             _SKIP_GOOGLE_DEFAULT_CREDS + \
@@ -528,7 +528,7 @@ class PHP7Language:
         return {}
 
     def unimplemented_test_cases(self):
-        return _SKIP_COMPRESSION + \
+        return _SKIP_SERVER_COMPRESSION + \
             _SKIP_DATA_FRAME_PADDING + \
             _SKIP_SPECIAL_STATUS_MESSAGE + \
             _SKIP_GOOGLE_DEFAULT_CREDS + \