Issue 6221 - Add native tests.

Issue 6221 - Add native tests.

[hub, 2018-01-26 20:50:41]

This is a "draft". I added google test as a dependency (the exact same as
libadblockplus) and wrote some test for the bases classes.

[sergei, 2018-01-26 22:13:36]

IMO it's fine if we simply brush it a bit and land and grow the tests and edge
cases later, because it would be good to have the bare minimum from which we can
begin ASAP.

https://codereview.adblockplus.org/29680720/diff/29680721/meson.build
File meson.build (right):

https://codereview.adblockplus.org/29680720/diff/29680721/meson.build#newcode212
meson.build:212: cpp_args: [ '-isystem', join_paths(GTEST_DIR, 'include'),
I'm afraid isystem is not supported on windows, but we can add a branch here
later I guess, or even fix in meson.

For -pthread there should be a branch, can be done later too.

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/Strin...
File test/compiled/String.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/Strin...
test/compiled/String.cpp:3: 
Could you please remove the empty lines and add the license header here?

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/Strin...
File test/compiled/StringMap.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/Strin...
test/compiled/StringMap.cpp:2: #include <string>
Could you please remove the empty lines and add the license header here?

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/abpte...
File test/compiled/abptest.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/abpte...
test/compiled/abptest.cpp:2: #include "gtest/gtest.h"
Could you please remove the empty lines and add the license header here?

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/abpte...
test/compiled/abptest.cpp:6: TEST(TestRegexp, RegExp)
I would prefer to have it in a separate file.

[hub, 2018-01-27 03:33:38]

https://codereview.adblockplus.org/29680720/diff/29680721/meson.build
File meson.build (right):

https://codereview.adblockplus.org/29680720/diff/29680721/meson.build#newcode212
meson.build:212: cpp_args: [ '-isystem', join_paths(GTEST_DIR, 'include'),
On 2018/01/26 22:13:36, sergei wrote:
> I'm afraid isystem is not supported on windows, but we can add a branch here
> later I guess, or even fix in meson.
> 
> For -pthread there should be a branch, can be done later too.

I have reworked this part. I use the meson syntax to specify includes, it should
work everywhere.

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/Strin...
File test/compiled/String.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/Strin...
test/compiled/String.cpp:3: 
On 2018/01/26 22:13:36, sergei wrote:
> Could you please remove the empty lines and add the license header here?

Done.

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/Strin...
File test/compiled/StringMap.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/Strin...
test/compiled/StringMap.cpp:2: #include <string>
On 2018/01/26 22:13:36, sergei wrote:
> Could you please remove the empty lines and add the license header here?

Done.

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/abpte...
File test/compiled/abptest.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/abpte...
test/compiled/abptest.cpp:2: #include "gtest/gtest.h"
On 2018/01/26 22:13:36, sergei wrote:
> Could you please remove the empty lines and add the license header here?

Done.

https://codereview.adblockplus.org/29680720/diff/29680721/test/compiled/abpte...
test/compiled/abptest.cpp:6: TEST(TestRegexp, RegExp)
On 2018/01/26 22:13:36, sergei wrote:
> I would prefer to have it in a separate file.

Done.

[sergei, 2018-01-29 13:38:44]

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/RegEx...
File test/compiled/RegExp.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/RegEx...
test/compiled/RegExp.cpp:26: EXPECT_TRUE(TestRegExp(id, u"1234"_str));
I think, formally we should delete the regular expression at the end.

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
File test/compiled/String.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
test/compiled/String.cpp:31: TEST(TestString, OwnedString)
It's not important for this change set but in general I would rather prefer
something like
TEST(OwnedString, ConstructingInvalidString)

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
File test/compiled/StringMap.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
test/compiled/StringMap.cpp:30: map[u"Foobar"_str] = std::string("one");
just wonder, is it required to use `std::string("one")` and one cannot use
merely "one"?

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
test/compiled/StringMap.cpp:55: //EXPECT_EQ(map.size(), 2);
it should be compared against 3, not 2.

[hub, 2018-01-29 16:57:46]

Updated patch

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/RegEx...
File test/compiled/RegExp.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/RegEx...
test/compiled/RegExp.cpp:26: EXPECT_TRUE(TestRegExp(id, u"1234"_str));
On 2018/01/29 13:38:44, sergei wrote:
> I think, formally we should delete the regular expression at the end.

Done.

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
File test/compiled/String.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
test/compiled/String.cpp:31: TEST(TestString, OwnedString)
On 2018/01/29 13:38:44, sergei wrote:
> It's not important for this change set but in general I would rather prefer
> something like
> TEST(OwnedString, ConstructingInvalidString)

Done.

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
File test/compiled/StringMap.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
test/compiled/StringMap.cpp:30: map[u"Foobar"_str] = std::string("one");
On 2018/01/29 13:38:44, sergei wrote:
> just wonder, is it required to use `std::string("one")` and one cannot use
> merely "one"?

constructor is implicit, so it is not needed.

https://codereview.adblockplus.org/29680720/diff/29680727/test/compiled/Strin...
test/compiled/StringMap.cpp:55: //EXPECT_EQ(map.size(), 2);
On 2018/01/29 13:38:44, sergei wrote:
> it should be compared against 3, not 2.

forgot to change that one.

Done

[sergei, 2018-01-30 13:05:37]

LGTM

https://codereview.adblockplus.org/29680720/diff/29683682/test/compiled/RegEx...
File test/compiled/RegExp.cpp (right):

https://codereview.adblockplus.org/29680720/diff/29683682/test/compiled/RegEx...
test/compiled/RegExp.cpp:30: EXPECT_FALSE(TestRegExp(id, u"abcd"_str));
I'm not sure that it should work this way, but OK, I guess it will be changes
later anyway.