Added test files for SystemInfo.m and OmahaXMLRequest.m.

chrome/installer/mac/app/testing/OmahaXMLRequest_test.mm

+// Copyright 2016 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 "../OmahaXMLRequest.h"

[Mark Mentovai, 2016/07/15 21:56:08]

#import this one. Also, we should probably move to more absolute paths for
#includes.

[Anna Zeng, 2016/07/22 16:58:02]

Done.

[Anna Zeng, 2016/07/22 16:58:02]

Acknowledged.

+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class OmahaXMLRequestTest : public ::testing::Test {
+ protected:
+  OmahaXMLRequestTest() : Test() {
+    xmlBody = [OmahaXMLRequest createXMLRequestBody];
+  }
+
+  virtual ~OmahaXMLRequestTest() { [xmlBody release]; }

[Mark Mentovai, 2016/07/15 21:56:08]

Here’s where scoped_nsobject would be handy: you won’t need to write your own
releases in destructors if you’re using scoped_nsobject<NSXMLDocument> xmlBody_;

[Anna Zeng, 2016/07/22 16:58:01]

Done.

[Anna Zeng, 2016/07/22 16:58:02]

Acknowledged.

+
+  NSXMLDocument* xmlBody;

[Mark Mentovai, 2016/07/15 21:56:08]

Remember to use trailing underscores for your member variables. Also, use
C++-style naming, which is underscores separating words: xml_body_.
https://google.github.io/styleguide/cppguide.html#Variable_Names

[Mark Mentovai, 2016/07/15 21:56:09]

This should be in a private: section.
https://google.github.io/styleguide/cppguide.html#Access_Control

[Anna Zeng, 2016/07/18 18:15:53]

From the above link: " For technical reasons, we allow data members of a test
fixture class to be protected when using Google Test)."

[Anna Zeng, 2016/07/22 16:58:01]

Done.

[Anna Zeng, 2016/07/22 16:58:02]

Acknowledged.

[Anna Zeng, 2016/07/22 16:58:02]

Acknowledged.

+};

[Mark Mentovai, 2016/07/15 21:56:08]

The private: section should also end with DISALLOW_COPY_AND_ASSIGN.
https://google.github.io/styleguide/cppguide.html#Declaration_Order

[Anna Zeng, 2016/07/22 01:45:37]

Since there is no private: section and this is test code, I will not include
this macro. Robert also mentioned that he seldom includes it as well.

[Anna Zeng, 2016/07/22 16:58:02]

Acknowledged.

+
+TEST_F(OmahaXMLRequestTest, createDoesNotReturnNil) {
+  EXPECT_NE(xmlBody, nil);

[Mark Mentovai, 2016/07/15 21:56:08]

EXPECT_TRUE(xml_body_) is fine.

[Mark Mentovai, 2016/07/15 21:56:08]

Not sure that you need this independent test case. createReturnsValidXML
effectively tests this too. If you want to be explicit about it, you can stick
EXPECT_TRUE(xml_body_) in as the first line of createReturnsValidXML.

[Mark Mentovai, 2016/07/22 15:42:38]

I wrote:
This comment from last time was unaddressed.

[Anna Zeng, 2016/07/22 16:43:21]

Oh oops! I recall addressing this somehow. Thanks for the reminder, this is now
addressed!

+}
+
+TEST_F(OmahaXMLRequestTest, createReturnsValidXML) {

[Mark Mentovai, 2016/07/15 21:56:08]

Test case names should begin with capital letters. This applies in this file and
in the system info test.

[Anna Zeng, 2016/07/22 16:58:02]

Done.

+  NSString* requestDTDLocation =
+      [[[NSFileManager defaultManager] currentDirectoryPath]
+          stringByAppendingString:@"/testing/requestCheck.dtd"];
+  NSData* requestDTDData = [NSData dataWithContentsOfFile:requestDTDLocation];
+  NSError* error;
+  NSXMLDTD* requestXMLChecker =
+      [[NSXMLDTD alloc] initWithData:requestDTDData options:0 error:&error];
+  [requestXMLChecker setName:@"request"];
+  [xmlBody setDTD:requestXMLChecker];
+  EXPECT_TRUE([xmlBody validateAndReturnError:&error]);
+}
+

[Mark Mentovai, 2016/07/15 21:56:08]

Do you want to parse the XML a bit more to make sure that it’s got the things
that we intended to set in it?

[Anna Zeng, 2016/07/22 01:45:37]

I'm able to currently guarantee lack of whitespace in attribute values and for
some, I can specify exactly what string the value should be. I can't use regular
expressions within the DTD files though. Would it be a good idea to include
regex to check the values of the attributes?

+}  // namespace