Issue #1467, #3397 - Rewrite the map from threads to tabs

test/plugin/InstancesTest.cpp

Index: test/plugin/InstancesTest.cpp
===================================================================
new file mode 100644
--- /dev/null
+++ b/test/plugin/InstancesTest.cpp
@@ -0,0 +1,69 @@
+/*
+ * This file is part of Adblock Plus <https://adblockplus.org/>,
+ * Copyright (C) 2006-2015 Eyeo GmbH
+ *
+ * Adblock Plus is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 3 as
+ * published by the Free Software Foundation.
+ *
+ * Adblock Plus is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <gtest/gtest.h>
+#include "../../src/plugin/Instances.h"
+
+typedef SyncMap<int, int, 0> SyncMapOne;

[sergei, 2016/02/01 15:50:43]

What does "One" mean?

[Eric, 2016/02/03 17:17:05]

It means its the first 'SyncMap' for testing. If we need another, we can call it
'SyncMapTwo'.

[sergei, 2016/02/08 13:35:36]

Let's solve the problem of name collision when it really happens.

Here it would be better to call it `IntSyncMap`.

[Eric, 2016/02/08 18:45:30]

Bikeshedding.

+
+TEST(SyncMap, Instantiate)

[sergei, 2016/02/08 13:35:36]

Where is the test "Destroy"?

I'm not sure that we need such test.

[Eric, 2016/02/08 18:45:30]

This test also calls the destructor.

+{
+  SyncMapOne s;
+}
+
+TEST(SyncMap, OrdinaryAdded)
+{
+  SyncMapOne s;
+  ASSERT_TRUE(s.AddIfAbsent(1, 11));

[sergei, 2016/02/08 13:35:36]

I still think that we should use EXPECT_ instead of ASSERT_.

[Eric, 2016/02/08 18:45:30]

'EXPECT' is inferior here, and it isn't just personal preference. If
'AddIfAbsent' fails, then it's assured that the other two calls later will fail.
That's two extra lines in the error log that provide no new information, but
that the developer still has to understand. It's a bad idea.

Rejected.

+  ASSERT_TRUE(s.Locate(1) == 11);
+  ASSERT_TRUE(s.RemoveAndCheck(1));
+}
+
+TEST(SyncMap, OrdinaryNotAdded1)
+{
+  SyncMapOne s;
+  ASSERT_TRUE(s.Locate(1) == 0);
+}
+
+TEST(SyncMap, OrdinaryNotAdded2)
+{
+  SyncMapOne s;
+  ASSERT_TRUE(s.AddIfAbsent(1, 11));
+  ASSERT_TRUE(s.AddIfAbsent(2, 22));
+  ASSERT_TRUE(s.Locate(7) == 0);
+}
+
+TEST(SyncMap, WrongAddedTwice)
+{
+  SyncMapOne s;
+  ASSERT_TRUE(s.AddIfAbsent(3, 11));
+  ASSERT_FALSE(s.AddIfAbsent(3, 22));
+}
+
+TEST(SyncMap, AcceptableAddedRemovedAddedAgain)
+{
+  SyncMapOne s;
+  ASSERT_TRUE(s.AddIfAbsent(1, 11));
+  ASSERT_TRUE(s.RemoveAndCheck(1));
+  ASSERT_TRUE(s.AddIfAbsent(1, 22));
+}
+
+TEST(SyncMap, WrongRemovedButNotAdded)
+{
+  SyncMapOne s;
+  ASSERT_FALSE(s.RemoveAndCheck(3));
+}
+