[20424] Dynamic language binding tests #4626

JLBuenoLopez · 2024-03-26T13:28:08Z

Description

This PR adds types that test the Dynamic Language Binding.

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
N/A Any new/modified methods have been properly documented using Doxygen.
Changes are ABI compatible.
Changes are API compatible.
N/A New feature has been added to the versions.md file (if applicable).
N/A New feature has been documented/Current behavior is correctly described in the documentation.
N/A Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
The title and description correctly express the PR's purpose.
Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

JLBuenoLopez · 2024-03-26T13:52:50Z

The following suggestions should be taken into account:

JLBuenoLopez

Partial review

JLBuenoLopez · 2024-03-26T15:25:07Z

test/feature/dynamic_types/DynamicTypesTests.cpp

+    {
+        eprosima::fastdds::dds::Log::ScopeLogs _("disable");
+        // Try to add a descriptor with the same name.
+        member_descriptor = traits<MemberDescriptor>::make_shared();
+        member_descriptor->type(factory->get_primitive_type(eprosima::fastdds::dds::TK_INT32));
+        member_descriptor->name("THIRD");
+        EXPECT_NE(builder->add_member(member_descriptor), RETCODE_OK);
+    }


I think we should add some more enum MemberDescriptor consistency checks apart from the non-empty name suggested here:

Use of default_value_ to give the enumerator a specific value different from the sequential one automatically assigned. Add another literal and check that the sequential order restarts with the previous literal value.

MemberId taking a value different from MEMBER_ID_INVALID: MemberDescriptor should be inconsistent.

is_default_label_ flag should be false according to the specification, but I think this field should be used to apply the @default_literal builtin annotation.

is_key_ flag should be false as this annotation only applies to Structure members and Union discriminators.

is_must_understand_ flag should be false as this builtin annotation only applies to Structure members.

is_optional_ flag should be false as this builtin annotation only applies to Structure members.

is_shared_ flag should be false as this annotation only applies to Structure members.

label property should be empty in order for the literal to be consistent.

try_construct_kind does not apply, but the enumerator which is defined in the IDL does not allow for this case. It is already initialized to DISCARD so I am not sure if a consistency check can be done with this property.

Done except default_value_ beucase that logic is not implemented yet.

test/feature/dynamic_types/DynamicTypesTests.cpp

JLBuenoLopez

Another DynamicTypesTests partial review

test/feature/dynamic_types/DynamicTypesTests.cpp

JLBuenoLopez · 2024-04-09T06:04:54Z

This PR should fix inheritance (both structure and bitsets) when loading a Dynamic Type from a XML profiles file.

JLBuenoLopez

Partial review

test/feature/dynamic_types/DynamicTypesTests.cpp

JLBuenoLopez · 2024-04-15T10:24:33Z

test/feature/dynamic_types/DynamicTypesTests.cpp

+    ASSERT_TRUE(struct_data);
+
+    // Test get_member_by_name and get_member_by_index.
+    ASSERT_EQ(MEMBER_ID_INVALID, struct_data->get_member_id_by_name(""));
+    ASSERT_EQ(0, struct_data->get_member_id_by_name("Structure"));
+    ASSERT_EQ(10, struct_data->get_member_id_by_name("int64"));
+    ASSERT_EQ(0, struct_data->get_member_id_at_index(0));
+    ASSERT_EQ(10, struct_data->get_member_id_at_index(1));
+


I think this is not correct. Getting the complex value for a non-selected union member fails. I would expect the same to happen for loans. And if we decide that the loan can be returned and if set, the discriminator points to the new union member, then the same philosophy must be followed with complex API.

Also, the test only uses the get API and this should fail because the union member being read is not the one selected.

test/feature/dynamic_types/DynamicTypesTests.cpp

JLBuenoLopez

Final review for DynamicTypesTests

test/feature/dynamic_types/DynamicTypesTests.cpp

JLBuenoLopez · 2024-04-16T08:38:07Z

XML structure and bitset inheritance must be fixed. Please, open also a documentation PR fixing the corresponding snippets.

JLBuenoLopez

Partial review. Only maps.idl tests pending for review.

test/feature/dynamic_types/DynamicTypesTests.cpp

test/feature/dynamic_types/dds_types_tests/DynamicTypesInheritanceDDSTypesTests.cpp

test/feature/dynamic_types/dds_types_tests/DynamicTypesMapsDDSTypesTests.cpp

JLBuenoLopez

Partial review

test/feature/dynamic_types/dds_types_tests/DynamicTypesMapsDDSTypesTests.cpp

JLBuenoLopez

Final review for my part.

test/feature/dynamic_types/dds_types_tests/DynamicTypesMapsDDSTypesTests.cpp

JLBuenoLopez · 2024-04-18T09:30:09Z

test/feature/dynamic_types/dds_types_tests/DynamicTypesMapsDDSTypesTests.cpp

+                static_pubsubType);
+        EXPECT_EQ(value.size(), struct_data.var_map_long_double().size());
+        for (auto const& map_element : value)
+


I have been rereading the specification and what it says is:

Map keys are implicitly converted to strings

But the specification does not specify which kind of string. So I suppose, that we should either define both APIs with std::string or just std::wstring. This change would introduce support to keys with wide string keys and wide string annotation parameters

Signed-off-by: adriancampo <adriancampo@eprosima.com> Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Signed-off-by: Ricardo González <ricardo@richiware.dev>

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

The base branch was changed.

JLBuenoLopez added this to the v3.0.0 milestone Mar 26, 2024

Base automatically changed from feature/dynamic-type-builder-factory-create-type-w-type-object-impl to feature/xtypes1.3 March 27, 2024 15:33

JLBuenoLopez commented Mar 27, 2024

View reviewed changes

JLBuenoLopez force-pushed the feature/tests_for_dds_types_test branch from 118fab0 to 059cd7a Compare April 1, 2024 05:43

richiware force-pushed the feature/xtypes1.3 branch from a574750 to e2458cb Compare April 2, 2024 06:17

JLBuenoLopez commented Apr 2, 2024

View reviewed changes

JLBuenoLopez force-pushed the feature/xtypes1.3 branch from e2458cb to 142d003 Compare April 2, 2024 07:24

richiware force-pushed the feature/xtypes1.3 branch from 142d003 to eb38f4c Compare April 2, 2024 08:01

JLBuenoLopez force-pushed the feature/tests_for_dds_types_test branch from c53092e to 3f62216 Compare April 3, 2024 07:37

richiware force-pushed the feature/xtypes1.3 branch from b92afc8 to bcc11eb Compare April 3, 2024 08:19

adriancampo force-pushed the feature/tests_for_dds_types_test branch from 3f62216 to fd95613 Compare April 3, 2024 10:05

richiware force-pushed the feature/xtypes1.3 branch from bcc11eb to 14e8cf5 Compare April 3, 2024 12:56

JLBuenoLopez mentioned this pull request Apr 4, 2024

Partial implementation of X-Types1.3 #4640

Merged

10 tasks

Base automatically changed from feature/xtypes1.3 to 3.0.x-devel April 5, 2024 05:39

richiware force-pushed the feature/tests_for_dds_types_test branch from 7aeac8e to 8096fd0 Compare April 10, 2024 13:03

richiware force-pushed the 3.0.x-devel branch from fb8e560 to 943b988 Compare April 11, 2024 13:01

JesusPoderoso force-pushed the 3.0.x-devel branch from 943b988 to 2c47b5e Compare April 12, 2024 05:50

richiware force-pushed the feature/tests_for_dds_types_test branch from f693187 to 2a37cf6 Compare April 12, 2024 10:47

JLBuenoLopez commented Apr 15, 2024

View reviewed changes

richiware mentioned this pull request Apr 15, 2024

[20631] Update xtypes 1.3 doc eProsima/Fast-DDS-docs#730

Merged

7 tasks

JLBuenoLopez commented Apr 15, 2024

View reviewed changes

github-actions bot force-pushed the 3.0.x-devel branch from 2c47b5e to c978d52 Compare April 16, 2024 08:56

JLBuenoLopez commented Apr 17, 2024

View reviewed changes

JLBuenoLopez commented Apr 18, 2024

View reviewed changes

EduPonz force-pushed the 3.0.x-devel branch 2 times, most recently from 8c0830d to 5148f5d Compare April 18, 2024 13:08

adriancampo and others added 15 commits May 13, 2024 11:35

Refs #20424: Added tests for finals and mutables.

e6cf58a

Signed-off-by: adriancampo <adriancampo@eprosima.com> Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424: Regenerate idl files

75127a6

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Fix tests

dac4a4e

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Apply suggestions 1

90c9ab2

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Apply suggestions 2

6132f8a

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Apply suggestions 3

dbed515

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Apply suggestions 4

e0e902c

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Apply suggestions 4.1

0e2d276

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Apply suggestions 5

489744f

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Reduce code

bd32eb9

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Fix warnings on windows

4f4e8d3

Signed-off-by: Ricardo González <ricardo@richiware.dev>

Refs #20424. Fix bigobj

f4e9183

Signed-off-by: Ricardo González <ricardo@richiware.dev>

Refs #20424. Fixing more windows warnings

f17aad6

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Fixing windows warning on generated code

a76b0a0

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

Refs #20424. Fixing windows warning on generated code

337a21b

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

richiware force-pushed the feature/tests_for_dds_types_test branch from 5220896 to 7801390 Compare May 13, 2024 09:42

richiware requested review from richiware and removed request for richiware May 13, 2024 09:42

Refs #20424. Fixing windows and mac warnings.

be61920

Signed-off-by: Ricardo González Moreno <ricardo@richiware.dev>

richiware force-pushed the feature/tests_for_dds_types_test branch from 7801390 to be61920 Compare May 13, 2024 10:22

richiware requested review from richiware and removed request for richiware May 13, 2024 10:22

richiware changed the base branch from 3.0.x-devel to master May 14, 2024 06:33

MiguelCompany force-pushed the master branch from 2e9ca54 to cee0599 Compare May 14, 2024 06:40

MiguelCompany changed the base branch from master to 3.0.x-devel May 14, 2024 07:01

MiguelCompany changed the base branch from 3.0.x-devel to master May 14, 2024 07:01

richiware approved these changes May 14, 2024

View reviewed changes

richiware merged commit bc03220 into master May 14, 2024
7 of 12 checks passed

richiware deleted the feature/tests_for_dds_types_test branch May 14, 2024 07:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[20424] Dynamic language binding tests #4626

[20424] Dynamic language binding tests #4626

JLBuenoLopez commented Mar 26, 2024 •

edited by richiware

Loading

JLBuenoLopez commented Mar 26, 2024 •

edited by richiware

Loading

JLBuenoLopez left a comment

JLBuenoLopez Mar 26, 2024

richiware Apr 23, 2024

JLBuenoLopez left a comment

JLBuenoLopez commented Apr 9, 2024

JLBuenoLopez left a comment

JLBuenoLopez Apr 15, 2024

JLBuenoLopez left a comment

JLBuenoLopez commented Apr 16, 2024

JLBuenoLopez left a comment

JLBuenoLopez left a comment

JLBuenoLopez left a comment

JLBuenoLopez Apr 18, 2024

[20424] Dynamic language binding tests #4626

[20424] Dynamic language binding tests #4626

Conversation

JLBuenoLopez commented Mar 26, 2024 • edited by richiware Loading

Description

Contributor Checklist

Reviewer Checklist

JLBuenoLopez commented Mar 26, 2024 • edited by richiware Loading

JLBuenoLopez left a comment

Choose a reason for hiding this comment

JLBuenoLopez Mar 26, 2024

Choose a reason for hiding this comment

richiware Apr 23, 2024

Choose a reason for hiding this comment

JLBuenoLopez left a comment

Choose a reason for hiding this comment

JLBuenoLopez commented Apr 9, 2024

JLBuenoLopez left a comment

Choose a reason for hiding this comment

JLBuenoLopez Apr 15, 2024

Choose a reason for hiding this comment

JLBuenoLopez left a comment

Choose a reason for hiding this comment

JLBuenoLopez commented Apr 16, 2024

JLBuenoLopez left a comment

Choose a reason for hiding this comment

JLBuenoLopez left a comment

Choose a reason for hiding this comment

JLBuenoLopez left a comment

Choose a reason for hiding this comment

JLBuenoLopez Apr 18, 2024

Choose a reason for hiding this comment

JLBuenoLopez commented Mar 26, 2024 •

edited by richiware

Loading

JLBuenoLopez commented Mar 26, 2024 •

edited by richiware

Loading