Skip to content

[tizen_rpc_port] Add integration tests and fix unsigned byte read#1054

Open
seungsoo47 wants to merge 7 commits into
flutter-tizen:masterfrom
seungsoo47:tizen_rpc_port-add-integration-tests
Open

[tizen_rpc_port] Add integration tests and fix unsigned byte read#1054
seungsoo47 wants to merge 7 commits into
flutter-tizen:masterfrom
seungsoo47:tizen_rpc_port-add-integration-tests

Conversation

@seungsoo47

@seungsoo47 seungsoo47 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Adds regression integration tests for the tizen_rpc_port plugin (both the client and server examples) and fixes an ABI-dependent bug uncovered by them.

Integration tests

  • Add Parcel primitive round-trip tests: writeBool/Int32/Int16/Int64/Byte,
    writeString, writeDouble, writeArrayCount, burst write/read, asRaw,
    Parcel.fromRaw, writeBundle, header (tag/sequenceNumber), and a custom
    Parcelable.
  • Include negative and boundary values to exercise sign handling across the FFI
    boundary: Int16 -1 / -32768, Int64 max / min, and Byte 255 / -1 / 8-bit
    masking.
  • Add PortType, ProxyBase, and StubBase observable-behavior tests.

Plugin fix

  • Parcel.readByte read the value into a C char, whose signedness is
    ABI-dependent (signed on x86, unsigned on ARM), so a byte written as 0xff
    read back as -1 on x86 while writeByte/read already use unsigned
    semantics. Mask the read value to 8 bits so readByte consistently returns
    0-255 on all targets. Bumps the version to 0.1.6.

Test

All tests pass on:

  • tv-9.0 emulator (x86)
  • tv-10.0 emulator (x86_64)
  • Raspberry Pi 4 (armv7 / ARM)

client: 30 added cases, server: 4 added cases.

#1039

* Parcel: round-trips for writeInt16/readInt16 (positive, zero, 16-bit masking)
* Parcel: round-trips for writeInt64/readInt64 (large value, zero)
* Parcel: round-trips for writeArrayCount/readArrayCount (non-zero, zero)
* Parcel: round-trips for write/read burst byte array (multi-byte, single byte)
* Parcel: asRaw returns non-empty bytes after write
* Parcel.fromRaw: reconstructed parcel reads back original values
* Parcel: writeBundle/readBundle round-trips (non-empty Bundle, empty Bundle)
* Parcel header: tag and sequenceNumber can be set and retrieved, tag defaults to empty string
* Parcel Parcelable: custom Parcelable serializes and deserializes correctly
* PortType: main and callback values are distinct; PortType.values contains both entries
* ProxyBase: isConnected is false before connecting; appid and portName are set from constructor
* StubBase: portName is set from constructor; setTrusted and addPrivilege work without listen

Note: listen()+close() lifecycle tests are excluded because rpc_port_stub_destroy()
does not safely interoperate with the internal listener thread started by
rpc_port_stub_listen() on armv7l; the resulting pthread mutex assertion is a
platform library limitation.
* Parcel: basic read/write for bool, int32, string, byte, double (pre-existing)
* Parcel: round-trips for writeInt16/readInt16 (positive, zero, 16-bit masking)
* Parcel: round-trips for writeInt64/readInt64 (large value, zero)
* Parcel: round-trips for writeArrayCount/readArrayCount (non-zero, zero)
* Parcel: round-trips for write/read burst byte array (multi-byte, single byte)
* Parcel: asRaw returns non-empty bytes after write
* Parcel.fromRaw: reconstructed parcel reads back original values
* Parcel: writeBundle/readBundle round-trips (non-empty Bundle, empty Bundle)
* Parcel header: tag and sequenceNumber can be set and retrieved, tag defaults to empty string
* PortType: main and callback values are distinct; PortType.values contains both entries
* StubBase: portName is set from constructor; setTrusted and addPrivilege work without listen
* Message: portName is 'Message'; services list is initially empty

Also adds test_driver/integration_test.dart (missing boilerplate) and adds
tizen_bundle as a direct dependency of the server example.

Note: listen()+close() lifecycle tests are excluded for the same reason as the
client example — rpc_port_stub_destroy() does not safely interoperate with the
internal listener thread started by rpc_port_stub_listen() on armv7l.
StubBase is a server-side concept, so its tests belong in the server
example, not the client. Move the 3 StubBase test cases from the client
example to the server example, and add a Parcelable serialization test
to the server example. Update TC counts in CHANGELOG (client 24 → 21,
server 23 → 24).
Parcel.readByte read the value into a C `char`, whose signedness is
ABI-dependent (signed on x86, unsigned on ARM), so a byte written as 0xff read
back as -1 on x86 while writeByte and the burst read() already use unsigned
semantics. Mask the read value to 8 bits so readByte consistently returns
0-255 on all targets.

Bump version to 0.1.6.
Extend the Parcel integration tests (client and server) with negative and
boundary-value round trips to exercise sign handling across the FFI boundary:
- writeInt16/readInt16: -1 and the int16 minimum (-32768).
- writeInt64/readInt64: the int64 maximum and minimum.
- writeByte/readByte: unsigned round trips including 255, a negative input
  read back as 255, and 8-bit masking.

Verified on the tv-9.0 (x86) and tv-10.0 (x86_64) emulators.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates Parcel.readByte to mask the returned value with _byteMax to ensure consistent unsigned byte behavior across different ABIs. It also adds integration tests to both the client and server example packages. The reviewer suggests addressing the duplication of over 200 lines of identical test code between the client and server integration test suites by either running them once or extracting them into a shared helper.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

The Parcel and PortType test groups are context-independent core-API tests and
were identical in the client and server example suites (~230 duplicated lines).
Keep them only in the client suite and leave the server suite with the
server-specific StubBase and Message tests, removing the redundant execution and
maintenance overhead.

@JSUYA JSUYA left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please delete tests that are semantically redundant.

Comment on lines +179 to +195
testWidgets('round-trips byte array', (WidgetTester _) async {
final Parcel parcel = Parcel();
final Uint8List bytes = Uint8List.fromList(
<int>[0x00, 0x01, 0x7f, 0x80, 0xff],
);
parcel.write(bytes);
final Uint8List result = parcel.read(bytes.length);
expect(result, equals(bytes));
});

testWidgets('round-trips single byte', (WidgetTester _) async {
final Parcel parcel = Parcel();
final Uint8List bytes = Uint8List.fromList(<int>[0xab]);
parcel.write(bytes);
expect(parcel.read(1), equals(bytes));
});
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two tests are nearly identical in content and looks redundant.

Comment on lines +293 to +304
testWidgets('main and callback values are distinct', (
WidgetTester _,
) async {
expect(PortType.main, isNot(equals(PortType.callback)));
});

testWidgets('PortType.values contains both entries', (
WidgetTester _,
) async {
expect(PortType.values,
containsAll(<PortType>[PortType.main, PortType.callback]));
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two tests are also similar.
Also, the purpose of the tests is ambiguous.
Are these tests to verify whether PortType.main and PortType.callback exist, respectively?

Comment on lines +197 to +223
group('asRaw', () {
testWidgets('returns non-empty bytes after write',
(WidgetTester _) async {
final Parcel parcel = Parcel();
parcel.writeInt32(1);
final Uint8List raw = parcel.asRaw();
expect(raw, isNotEmpty);
});
});

group('Parcel.fromRaw', () {
testWidgets('reconstructed parcel reads back original values', (
WidgetTester _,
) async {
final Parcel original = Parcel();
original.writeInt32(99);
original.writeString('raw');
original.writeBool(true);

final Uint8List raw = original.asRaw();
final Parcel restored = Parcel.fromRaw(raw);

expect(restored.readInt32(), 99);
expect(restored.readString(), 'raw');
expect(restored.readBool(), isTrue);
});
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two tests are also redundant. asRaw() exists inside fromRaw().

Comment on lines +67 to +77
testWidgets('round-trips positive value', (WidgetTester _) async {
final Parcel parcel = Parcel();
parcel.writeInt16(32767);
expect(parcel.readInt16(), 32767);
});

testWidgets('round-trips zero', (WidgetTester _) async {
final Parcel parcel = Parcel();
parcel.writeInt16(0);
expect(parcel.readInt16(), 0);
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any difference that needs to be verified internally when the value is zero or other?
It seems to overlap with several other tests of this type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants