-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
test/kubernetes_unittest.cc
Outdated
| {"pod_id", "TestUid"}, | ||
| {"zone", "TestZone"}, | ||
| }), m.resource()); | ||
| EXPECT_EQ("", m.metadata().version); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to:
EXPECT_TRUE(m.metadata().version.empty());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
| })} | ||
| }); | ||
| const auto m = GetLegacyResource(reader, json::object({ | ||
| {"metadata", json::object({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this just pod->As<json::Object>()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed pod, it was supposed to be inlined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually suggesting you keep pod...
| EXPECT_FALSE(m.metadata().is_deleted); | ||
| EXPECT_EQ(Timestamp(), m.metadata().created_at); | ||
| EXPECT_EQ(Timestamp(), m.metadata().collected_at); | ||
| EXPECT_TRUE(m.metadata().ignore); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Leave this one, remove all of the other checks of m.metadata() — they're implementation details.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
test/kubernetes_unittest.cc
Outdated
| EXPECT_EQ(Timestamp(), m.metadata().created_at); | ||
| EXPECT_EQ(Timestamp(), m.metadata().collected_at); | ||
| EXPECT_TRUE(m.metadata().ignore); | ||
| EXPECT_EQ(json::object({})->ToString(), m.metadata().metadata->ToString()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Irrelevant now, but json::object()->ToString() is just "{}".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, but removed as you pointed out.
test/kubernetes_unittest.cc
Outdated
| })->As<json::Object>(), "TestContainerName"); | ||
| EXPECT_EQ(std::vector<std::string>({ | ||
| "gke_container.TestNamespace.TestUid.TestContainerName", | ||
| "gke_container.TestNamespace.TestName.TestContainerName" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing comma, please.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added.
| })} | ||
| }); | ||
| const auto m = GetLegacyResource(reader, json::object({ | ||
| {"metadata", json::object({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually suggesting you keep pod...
igorpeshansky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ![]()
ea02f0f to
9fd6819
Compare
|
Rebased against master |
igorpeshansky
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ![]()
No description provided.