Issue #1234 - Add unit tests for TrimString

test/UtilTest.cpp

Index: test/UtilTest.cpp
===================================================================
new file mode 100644
--- /dev/null
+++ b/test/UtilTest.cpp
@@ -0,0 +1,105 @@
+/*
+ * This file is part of Adblock Plus <http://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/shared/Utils.h"
+
+//----------------------------------
+// Trim
+//----------------------------------

[sergei, 2015/01/05 15:27:43]

I would say we don't need these comments

[Eric, 2015/01/05 16:25:07]

We'll need these once the other unit tests land in this file. 

[sergei, 2015/01/05 16:51:03]

Why will we need it? We don't use them in other files with tests.

It may be even better to put other tests into another file.

[Eric, 2015/01/05 17:18:25]

I've got around another 70 tests already written (from the ATL conversion work a
few months ago) for various utility functions we'll need to finish off CString.
It's much easier to find the tests you're looking for with the separators.
The file structure for test we have mirrors that of the source structure. If we
had a build system that made it easier to deal with multiple test harnesses, I'd
be inclined to arrange the tests differently. You've got to compile the whole
executable with all the test sources anyway. As it is, though, I don't see any
value splitting out the tests into separate files at this point.

[sergei, 2015/01/05 21:13:53]

It still does not convince me, let's wait for @Oleksandr's or @Felix' response.

[Oleksandr, 2015/01/09 00:16:59]

If we add 70 more tests in this file it will be hardly readable. Comments like
this won't help much in that case, IMHO. I'd vote for having more files instead.

+namespace {

[sergei, 2015/01/05 15:27:43]

{ should be on the new line

[Eric, 2015/01/05 16:25:07]

Done.

+  void TrimTest( std::wstring input, std::wstring expected )
+  {
+    std::wstring trimmed = TrimString( input );
+    ASSERT_EQ( expected, trimmed );
+  }
+}
+
+TEST( Trim, trim_00 )

[sergei, 2015/01/05 15:27:43]

here, above and below: we don't use spaces after ( and before )

[Eric, 2015/01/05 16:25:07]

Done.

+{
+  TrimTest( L"", L"" );
+}
+
+TEST( Trim, trim_01 )
+{
+  TrimTest( L" ", L"" );
+}
+
+TEST( Trim, trim_02 )
+{
+  TrimTest( L"\n", L"" );
+}
+
+TEST( Trim, trim_03 )
+{
+  TrimTest( L"\r", L"" );
+}
+
+TEST( Trim, trim_04 )
+{
+  TrimTest( L"\t", L"" );
+}
+
+TEST( Trim, trim_05 )
+{
+  TrimTest( L"foo", L"foo" );
+}
+
+TEST( Trim, trim_06 )
+{
+  TrimTest( L" foo", L"foo" );
+}
+
+TEST( Trim, trim_07 )
+{
+  TrimTest( L"\r\nfoo", L"foo" );
+}
+
+TEST( Trim, trim_08 )
+{
+  TrimTest( L"\tfoo", L"foo" );
+}
+
+TEST( Trim, trim_09 )
+{
+  TrimTest( L"foo  ", L"foo" );
+}
+
+TEST( Trim, trim_10 )
+{
+  TrimTest( L"foo\r\n", L"foo" );
+}
+
+TEST( Trim, trim_11 )
+{
+  TrimTest( L"foo\t", L"foo" );
+}
+
+TEST( Trim, trim_12 )
+{
+  TrimTest( L"foo bar", L"foo bar" );
+}
+
+TEST( Trim, trim_13 )
+{
+  TrimTest( L"foo bar \r\n", L"foo bar" );
+}
+
+TEST( Trim, trim_14 )
+{
+  TrimTest( L"  foo bar \r\n", L"foo bar" );
+}
+