Add service protocol method to facilitate getting snapshots #32628
Conversation
iskakaushik
force-pushed
the
service-protocol
branch
from
1033a1e to
7d815b7
Compare
| rapidjson::Writer<rapidjson::StringBuffer> writer(buffer); | ||
| document.Accept(writer); | ||
| std::string expected_json = | ||
| "\"snapshot\":[137,80,78,71,13,10,26,10,0," |
There was a problem hiding this comment.
consider adding a doc comment explaining how this was generated and what might cause it to change (and what an acceptable change would be).
| "0,0,13,73,68,65,84,8,153,99,248,207,192,240,31,0,5,0,1,255,171,206,54," | ||
| "137,0,0,0,0,73,69,78,68,174,66,96,130]"; | ||
| std::string actual_json = buffer.GetString(); | ||
| EXPECT_THAT(actual_json, ::testing::HasSubstr(expected_json)); |
There was a problem hiding this comment.
Are there JSON methods we could be using instead to validate fields? e.g. validate that snapshot is a list of integers, and validate that the timing field is present?
There was a problem hiding this comment.
I could maybe parse it out with rapidjson and validate that the timing field exists. I was being lazy and using HasSubstring. Let me change it to do real parsing instead.
There was a problem hiding this comment.
(I'm mainly worried that this test will break if we move to a different rendering backend or the rendering backend slightly changes things and it'll take more work to validate)
|
I improved the test and added a comment on how to make it even better. |
iskakaushik
added
waiting for tree to go green
No description provided.