sync: add more integration tests for wifi_sync

chrome/browser/sync/test/integration/single_client_wifi_credentials_sync_test.cc

Index: chrome/browser/sync/test/integration/single_client_wifi_credentials_sync_test.cc
diff --git a/chrome/browser/sync/test/integration/single_client_wifi_credentials_sync_test.cc b/chrome/browser/sync/test/integration/single_client_wifi_credentials_sync_test.cc
index f8c68aa85ee5442ed0fc8d3004451227ee119b84..27e50b37858791ddb7915804b56543071e7e9e15 100644
--- a/chrome/browser/sync/test/integration/single_client_wifi_credentials_sync_test.cc
+++ b/chrome/browser/sync/test/integration/single_client_wifi_credentials_sync_test.cc
@@ -4,19 +4,34 @@
 
 #include "base/command_line.h"
 #include "base/macros.h"
+#include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 #include "chrome/browser/sync/test/integration/wifi_credentials_helper.h"
 #include "chrome/common/chrome_switches.h"
+#include "components/wifi_sync/wifi_credential.h"
+#include "components/wifi_sync/wifi_security_class.h"
+
+using sync_integration_test_util::AwaitCommitActivityCompletion;
+using wifi_sync::WifiCredential;
 
 class SingleClientWifiCredentialsSyncTest : public SyncTest {
  public:
   SingleClientWifiCredentialsSyncTest() : SyncTest(SINGLE_CLIENT) {}
   ~SingleClientWifiCredentialsSyncTest() override {}
 
+  // SyncTest implementation.
+  void SetUp() override {
+    wifi_credentials_helper::SetUp();
+    SyncTest::SetUp();
+  }
   void SetUpCommandLine(base::CommandLine* command_line) override {
     SyncTest::SetUpCommandLine(command_line);
     command_line->AppendSwitch(switches::kEnableWifiCredentialSync);
   }
+  bool SetupClients() override {
+    return SyncTest::SetupClients() &&
+        wifi_credentials_helper::SetupClients();
+  }
 
  private:
   DISALLOW_COPY_AND_ASSIGN(SingleClientWifiCredentialsSyncTest);
@@ -27,3 +42,19 @@ IN_PROC_BROWSER_TEST_F(SingleClientWifiCredentialsSyncTest, NoCredentials) {
   ASSERT_TRUE(wifi_credentials_helper::VerifierIsEmpty());
   ASSERT_TRUE(wifi_credentials_helper::ProfileMatchesVerifier(0));
 }
+
+IN_PROC_BROWSER_TEST_F(SingleClientWifiCredentialsSyncTest, SingleCredential) {

[pneubeck (no reviews), 2015/01/30 14:38:45]

is there a reason why this is a browser test but the file doesn't have the
suffix _browsertest.cc ?

[mukesh agrawal, 2015/01/30 22:28:14]

Just following the convention in this part of the tree. (All of the subclasses
of SyncTest have browser tests, and none are named _browsertest.cc.)

+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+
+  scoped_ptr<WifiCredential> credential =
+      wifi_credentials_helper::MakeWifiCredential(
+          "fake-ssid", wifi_sync::SECURITY_CLASS_PSK, "fake_passphrase");
+  ASSERT_TRUE(credential);
+
+  const size_t profile_index = 0;
+  wifi_credentials_helper::AddWifiCredential(
+      profile_index, "fake_id", *credential);
+  ASSERT_TRUE(AwaitCommitActivityCompletion(GetSyncService((0))));
+  EXPECT_FALSE(wifi_credentials_helper::VerifierIsEmpty());
+  EXPECT_TRUE(wifi_credentials_helper::ProfileMatchesVerifier(0));

[pneubeck (no reviews), 2015/01/30 14:38:45]

to not rely on some function that compares all profiles (and which might have
some bug itself), you could 'manually' compare the content in profile_b with the
credential above.
i.e. something like
 EXPECT_EQ(1, GetCredentialsForProfile(profile_b_index).size());
  EXPECT_EQ("fake-ssid", GetCredentialsForProfile(profile_b_index)[0]->ssid);

[mukesh agrawal, 2015/01/30 22:28:14]

I could, but that would diverge from the convention for sync integration tests.
(I believe they'are all structured to factor the comparison logic out into a
helper file.)

Apart from consistent code structuring, the helper method also provides error
messages that are consistent with other data types.

[pneubeck (no reviews), 2015/01/31 10:26:40]

my point was more about what you're comparing with. I don't see that you're
comparing with |credential| directly. Instead there is additional code involved
that must correctly store the credential in the verifier.

[mukesh agrawal, 2015/02/02 20:36:40]

AIUI, the verifier is actually an important part of the test framework. It's a
SyncableService, but does not participate in the sync protocol. It provides
protection against the case where all sync clients converge to a consistent, but
incorrect, state.

I've taken your suggestion, and applied it to the verifier. (i.e., first check
that the verifier got the credential, then check profiles against the verifier.)

+}