Skip to content

Commit 9079ef8

Browse files
Alexander Wilsonfacebook-github-bot
authored andcommitted
Fix Appending to Default in thrift::from_dynamic
Summary: Related post: https://fb.workplace.com/groups/thriftusers/posts/1722521021790636 tl;dr The `struct_with_list_field_with_default_values` test will fail, which is unexpected: ``` rdtscp@devvm34082•~/fbcode» hg diff thrift/lib/cpp2/folly_dynamic/internal/folly_dynamic-inl-post.h ; buck2 test thrift/lib/cpp2/folly_dynamic/test:folly_dynamic_test diff --git a/fbcode/thrift/lib/cpp2/folly_dynamic/internal/folly_dynamic-inl-post.h b/fbcode/thrift/lib/cpp2/folly_dynamic/internal/folly_dynamic-inl-post.h --- a/fbcode/thrift/lib/cpp2/folly_dynamic/internal/folly_dynamic-inl-post.h +++ b/fbcode/thrift/lib/cpp2/folly_dynamic/internal/folly_dynamic-inl-post.h @@ -163,7 +163,7 @@ if (input.empty()) { return; } - out.clear(); + // out.clear(); do_reserve(out, input.size()); for (const auto& i : input) { out.emplace_back(); ✗ Fail: thrift/lib/cpp2/folly_dynamic/test:folly_dynamic_test - FromDynamic.struct_with_list_field_with_default_values (0.0s) Note: Google Test filter = FromDynamic.struct_with_list_field_with_default_values [==========] Running 1 test from 1 test suite. [----------] Global test environment set-up. [----------] 1 test from FromDynamic [ RUN ] FromDynamic.struct_with_list_field_with_default_values fbcode/thrift/lib/cpp2/folly_dynamic/test/folly_dynamic_test.cpp:803: Failure Expected equality of these values: res.list_field().value() Which is: { 1, 2, 3, 4, 5, 6, 7 } std::vector<int>({4, 5, 6, 7}) Which is: { 4, 5, 6, 7 } [ FAILED ] FromDynamic.struct_with_list_field_with_default_values (1 ms) [----------] 1 test from FromDynamic (1 ms total) [----------] Global test environment tear-down [==========] 1 test from 1 test suite ran. (1 ms total) [ PASSED ] 0 tests. [ FAILED ] 1 test, listed below: [ FAILED ] FromDynamic.struct_with_list_field_with_default_values 1 FAILED TEST Buck UI: https://www.internalfb.com/buck2/41d891b6-ef9b-4c45-802f-c650a296a203 Test UI: https://www.internalfb.com/intern/testinfra/testrun/14355223924851959 Network: Up: 0B Down: 0B Command: test. Time elapsed: 3.0s Test execution completed but the tests failed Tests finished: Pass 23. Fail 1. Fatal 0. Skip 0. Build failure 0 1 TESTS FAILED ✗ thrift/lib/cpp2/folly_dynamic/test:folly_dynamic_test - FromDynamic.struct_with_list_field_with_default_values Run $ fdb buck test <args> to debug thrift/lib/cpp2/folly_dynamic/test:folly_dynamic_test - FromDynamic.struct_with_list_field_with_default_values ^^^ just prefix your previous command! ($ fdb !!) Learn more at https://fburl.com/fdb Use Devmate to fix your errors. Just click the following link: fb-vscode://nuclide.code-compose/devmate/fixtests?run_id=14355223924851959 ``` Reviewed By: Mizuchi, thedavekwon Differential Revision: D82813298 fbshipit-source-id: 81864aafbe8743fccd966cd85ad4bd5ddcda9aac
1 parent 6635a40 commit 9079ef8

File tree

3 files changed

+14
-0
lines changed

3 files changed

+14
-0
lines changed

thrift/lib/cpp2/folly_dynamic/internal/folly_dynamic-inl-post.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,7 @@ struct dynamic_converter_impl<apache::thrift::type::list<VTag>> {
163163
if (input.empty()) {
164164
return;
165165
}
166+
out.clear();
166167
do_reserve(out, input.size());
167168
for (const auto& i : input) {
168169
out.emplace_back();

thrift/lib/cpp2/folly_dynamic/test/folly_dynamic_test.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,3 +793,12 @@ TEST(FromDynamic, struct_with_vector_bool) {
793793
v, dynamic_format::PORTABLE, format_adherence::LENIENT);
794794
EXPECT_EQ(res.values(), std::vector<bool>({true, false, true, false}));
795795
}
796+
797+
TEST(FromDynamic, struct_with_list_field_with_default_values) {
798+
folly::dynamic v = folly::dynamic::object();
799+
v["list_field"] = folly::dynamic::array(4, 5, 6, 7);
800+
auto res = from_dynamic<
801+
test_cpp2::cpp_reflection::StructWithListFieldWithDefaultValues>(
802+
v, dynamic_format::PORTABLE, format_adherence::STRICT);
803+
EXPECT_EQ(res.list_field().value(), std::vector<int>({4, 5, 6, 7}));
804+
}

thrift/test/reflection/reflection.thrift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -675,3 +675,7 @@ struct StructWithAdaptedField {
675675
struct StructWithVectorBool {
676676
1: list<bool> values;
677677
}
678+
679+
struct StructWithListFieldWithDefaultValues {
680+
1: list<i32> list_field = [1, 2, 3];
681+
}

0 commit comments

Comments
 (0)