Skip to content

[tizen_package_manager] Add integration tests and fix icon path handling#1055

Open
seungsoo47 wants to merge 6 commits into
flutter-tizen:masterfrom
seungsoo47:tizen_package_manager-add-integration-tests
Open

[tizen_package_manager] Add integration tests and fix icon path handling#1055
seungsoo47 wants to merge 6 commits into
flutter-tizen:masterfrom
seungsoo47:tizen_package_manager-add-integration-tests

Conversation

@seungsoo47

@seungsoo47 seungsoo47 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Adds regression integration tests for the tizen_package_manager plugin and fixes package icon path handling.

Plugin fix

  • Return a null icon path (instead of an empty string) when a package has no icon, and only use package_info_get_icon's result when the call succeeds (PACKAGE_MANAGER_ERROR_NONE) and the path is non-empty; always free the returned buffer. The icon is optional, so a failure is not treated as fatal.
  • Bumps the version to 0.4.3.

Integration tests

  • Add tests for getPackageInfo, getPackageSizeInfo, getPackagesInfo, package/size-info field validation, error paths (empty / invalid IDs), and the install/uninstall/update event streams.

Test

All tests pass on:

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

#1039

- PackageInfo fields: installedStorageType is a valid StorageType value
- PackageInfo fields: iconPath is null or a non-empty string
- PackageSizeInfo fields: all size fields are non-negative
- getPackagesInfo list items: each PackageInfo item has valid field types
- getPackagesInfo list items: getPackagesInfo contains current package
- getPackageInfo error paths: throws ArgumentError for empty packageId
- getPackageInfo error paths: throws PlatformException for invalid packageId
- getPackageSizeInfo error paths: throws ArgumentError for empty packageId
- install error paths: throws ArgumentError for empty packagePath
- uninstall error paths: throws ArgumentError for empty packageId
- event streams: onInstallProgressChanged can be listened to without error
- event streams: onUninstallProgressChanged can be listened to without error
- event streams: onUpdateProgressChanged can be listened to without error
Only use the icon path from package_info_get_icon when the call succeeds
(PACKAGE_MANAGER_ERROR_NONE) and the path is non-empty, and always free the
returned buffer. The icon is optional, so a failure is not treated as fatal;
this also simplifies the previous branchy free logic.

Bump version to 0.4.3.
…ests

Await the error-path expectations with expectLater and use the built-in
throwsArgumentError matcher, so the asynchronous ArgumentError/PlatformException
throws are actually asserted (the previous unawaited expect() form could pass
without verifying the error). Convert the remaining plain test() cases to
testWidgets for consistency with the rest of the suite.

@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 the tizen_package_manager package to version 0.4.3, modifying the C++ implementation to safely handle optional icon paths by ignoring failures and empty paths. It also adds comprehensive integration tests for package info fields, error paths, and event streams. The review feedback suggests improving the tests by replacing conditional checks with the declarative anyOf(isNull, isNotEmpty) matcher to prevent silent passes, and removing redundant type assertions that are already statically guaranteed by Dart.

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.

…tests

Use anyOf(isNull, isNotEmpty) to assert the optional icon path declaratively,
and drop the isA<String>/isA<bool> and enum-in-values assertions in the
getPackagesInfo item test: PackageInfo.fromMap already casts every field (and
maps enums), so those types are statically guaranteed and the assertions
verified nothing. Only the value-level checks (non-empty id/version, icon path)
remain.
Comment on lines +48 to +50
final PackageInfo info =
await PackageManager.getPackageInfo(currentPackageId);
expect(StorageType.values, contains(info.installedStorageType));

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.

I think this test is invalid because the type of info.installedStorageType is StorageType and it must return one of them.

Comment on lines +137 to +176
group('install error paths', () {
testWidgets('throws ArgumentError for empty packagePath', (
WidgetTester tester,
) async {
await expectLater(PackageManager.install(''), throwsArgumentError);
});
});

group('uninstall error paths', () {
testWidgets('throws ArgumentError for empty packageId', (
WidgetTester tester,
) async {
await expectLater(PackageManager.uninstall(''), throwsArgumentError);
});
});

group('event streams', () {
testWidgets('onInstallProgressChanged can be listened to without error', (
WidgetTester tester,
) async {
final StreamSubscription<PackageEvent> subscription =
PackageManager.onInstallProgressChanged.listen(null);
await subscription.cancel();
});

testWidgets('onUninstallProgressChanged can be listened to without error', (
WidgetTester tester,
) async {
final StreamSubscription<PackageEvent> subscription =
PackageManager.onUninstallProgressChanged.listen(null);
await subscription.cancel();
});

testWidgets('onUpdateProgressChanged can be listened to without error', (
WidgetTester tester,
) async {
final StreamSubscription<PackageEvent> subscription =
PackageManager.onUpdateProgressChanged.listen(null);
await subscription.cancel();
});

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.

This test does not have an except process. Is it simply checking whether it works? If it is an await, you must check the result through a timeout or separate processing and verify the successful call status.

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