Adding 'Show Host List' option to CTD

remoting/test/chromoting_test_driver_environment.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/test/chromoting_test_driver_environment.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 68 matching lines @@
   if (!RetrieveAccessToken(auth_code)) {
     // If we cannot retrieve an access token, then nothing is going to work.
     // Let the caller know that our object is not ready to be used.
     return false;
   }
 
   return true;
 }
 
 void ChromotingTestDriverEnvironment::DisplayHostList() {
-  const char kHostAvailabilityFormatString[] = "%-45s%-15s%-35s";
+  const char kHostAvailabilityFormatString[] = "%-25s%-15s%-35s\n";
 
-  LOG(INFO) << base::StringPrintf(kHostAvailabilityFormatString,
-                                  "Host Name", "Host Status", "Host JID");
-  LOG(INFO) << base::StringPrintf(kHostAvailabilityFormatString,
-                                  "---------", "-----------", "--------");
+  printf(kHostAvailabilityFormatString, "Host Name", "Host Status", "Host JID");

[garykac, 2017/03/03 21:01:58]

Is the switch to printf intentional?

[joedow, 2017/03/03 22:45:59]

It was intentional.  Using LOG will prepend a bunch of info that makes the table
less readable IMO and less likely to fit within the a standard command window
without resizing.

+  printf(kHostAvailabilityFormatString, "---------", "-----------", "--------");
 
   std::string status;
   for (const HostInfo& host_info : host_list_) {
     HostStatus host_status = host_info.status;
     if (host_status == kHostStatusOnline) {
       status = "ONLINE";
     } else if (host_status == kHostStatusOffline) {
       status = "OFFLINE";
     } else {
       status = "UNKNOWN";
     }
 
-    LOG(INFO) << base::StringPrintf(
-        kHostAvailabilityFormatString, host_info.host_name.c_str(),
-        status.c_str(), host_info.host_jid.c_str());
+    printf(kHostAvailabilityFormatString, host_info.host_name.c_str(),
+           status.c_str(), host_info.host_jid.c_str());
   }
 }
 
-bool ChromotingTestDriverEnvironment::WaitForHostOnline(
-    const std::string& host_jid,
-    const std::string& host_name) {
+bool ChromotingTestDriverEnvironment::WaitForHostOnline() {
   if (host_list_.empty()) {
     RetrieveHostList();
   }
 
+  DisplayHostList();
+
   // Refresh the |host_list_| periodically to check if expected JID is online.
   const base::TimeDelta kTotalTimeInSeconds = base::TimeDelta::FromSeconds(60);
   const base::TimeDelta kSleepTimeInSeconds = base::TimeDelta::FromSeconds(5);
   const int kMaxIterations = kTotalTimeInSeconds / kSleepTimeInSeconds;
 
   int num_iterations = 0;

[garykac, 2017/03/03 21:01:58]

No longer used.

[joedow, 2017/03/03 22:45:59]

Good catch

-  while (num_iterations < kMaxIterations) {
+  for (int iterations = 0; iterations < kMaxIterations; iterations++) {
+    if (!FindHostInHostList()) {
+      LOG(WARNING) << "Host '" << host_name_ << "' with JID '" << host_jid_
+                   << "' not found in host list.";
+      return false;
+    }
+
     if (host_info_.IsReadyForConnection()) {
-      if (num_iterations > 0) {
+      if (iterations > 0) {
         VLOG(0) << "Host online after: "
-                << num_iterations * kSleepTimeInSeconds.InSeconds()
-                << " seconds.";
+                << iterations * kSleepTimeInSeconds.InSeconds() << " seconds.";
       }
       return true;
     }
 
     // Wait a while before refreshing host list.
     base::PlatformThread::Sleep(kSleepTimeInSeconds);
     RefreshHostList();
-    ++num_iterations;
   }
 
-  LOG(ERROR) << "Host with JID '" << host_jid << "' still not online after "
+  LOG(ERROR) << "Host '" << host_name_ << "' with JID '" << host_jid_
+             << "' still not online after "
              << num_iterations * kSleepTimeInSeconds.InSeconds() << " seconds.";
   return false;
 }
 
+bool ChromotingTestDriverEnvironment::FindHostInHostList() {
+  bool host_found = false;
+  for (HostInfo& host_info : host_list_) {
+    // The JID is optional so we consider an empty string to be a '*' match.
+    bool host_jid_match =
+        host_jid_.empty() || (host_jid_ == host_info.host_jid);
+    bool host_name_match = host_name_ == host_info.host_name;
+
+    if (host_name_match && host_jid_match) {
+      host_info_ = host_info;
+      host_found = true;
+      break;
+    }
+  }
+  return host_found;
+}
+
 void ChromotingTestDriverEnvironment::SetAccessTokenFetcherForTest(
     AccessTokenFetcher* access_token_fetcher) {
   DCHECK(access_token_fetcher);
 
   test_access_token_fetcher_ = access_token_fetcher;
 }
 
 void ChromotingTestDriverEnvironment::SetRefreshTokenStoreForTest(
     RefreshTokenStore* refresh_token_store) {
   DCHECK(refresh_token_store);
@@ skipping 142 matching lines @@
 
   run_loop.Run();
 
   if (host_list_.empty()) {
     // Note: Access token may have expired, but it is unlikely.
     LOG(ERROR) << "Retrieved host list is empty.\n"
                << "Does the account have hosts set up?";
     return false;
   }
 
-  DisplayHostList();
-  for (HostInfo& host_info : host_list_) {
-    // The JID is optional so we consider an empty string to be a '*' match.
-    bool host_jid_match =
-        host_jid_.empty() || (host_jid_ == host_info.host_jid);
-    bool host_name_match = host_name_ == host_info.host_name;
-
-    if (host_name_match && host_jid_match) {
-      host_info_ = host_info;
-
-      if (host_info.IsReadyForConnection()) {
-        return true;
-      } else {
-        LOG(WARNING) << "Host '" << host_name_ << "' with JID '" << host_jid_
-                     << "' not online.";
-        return false;
-      }
-    }
-  }
-  LOG(WARNING) << "Host '" << host_name_ << "' with JID '" << host_jid_
-               << "' not found in host list.";
-  return false;
+  return true;
 }
 
 void ChromotingTestDriverEnvironment::OnHostListRetrieved(
     base::Closure done_closure,
     const std::vector<HostInfo>& retrieved_host_list) {
   VLOG(1) << "OnHostListRetrieved() Called";
 
   host_list_ = retrieved_host_list;
 
   done_closure.Run();
 }
 
 }  // namespace test
 }  // namespace remoting