Feature/new wrapper test added #15
Conversation
- Remove duplicate bidder log line from onAdLoaded callbacks - Add placementName, placementId, and externalPlacementId to logs - Rename "Bidder" to "Bidder/Network" for clarity - Apply changes to banner, MREC, and interstitial screens This provides complete ad metadata visibility for testing and debugging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a comprehensive testing application for the CloudX Flutter SDK wrapper validation. The main purpose is to provide a structured test environment to verify that the Flutter wrapper correctly integrates with the native CloudX SDK on both iOS and Android platforms.
Key changes:
- Adds a new
cloudx-flutter-testingdirectory with a complete Flutter test application - Updates the demo app's CloudX Flutter dependency from ^0.17.0 to ^0.18.0
- Implements a clean architecture approach with proper separation of concerns for testing
Reviewed Changes
Copilot reviewed 51 out of 77 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| cloudx_flutter_demo_app/pubspec.yaml | Updates CloudX Flutter SDK dependency to version 0.18.0 |
| cloudx-flutter-testing/pubspec.yaml | Defines dependencies for the new testing app with local SDK path |
| cloudx-flutter-testing/test/widget_test.dart | Adds a basic widget test for counter functionality |
| cloudx-flutter-testing/lib/main.dart | Implements main app entry point with tab-based navigation |
| cloudx-flutter-testing/lib/services/sdk_manager.dart | Creates SDK initialization and management service |
| cloudx-flutter-testing/lib/screens/*.dart | Implements screens for settings, initialization, and ad testing |
| cloudx-flutter-testing/ios/* | Adds complete iOS project configuration |
| cloudx-flutter-testing/android/* | Adds complete Android project configuration |
| cloudx-flutter-testing/README.md | Provides comprehensive documentation for the testing app |
| cloudx-flutter-testing/ARCHITECTURE.md | Describes the clean architecture approach and testing strategy |
| README.md | Updates root README to reference the new testing app |
Files not reviewed (2)
- cloudx-flutter-testing/ios/Runner.xcodeproj/project.xcworkspace/contents.xcworkspacedata: Language not supported
- cloudx-flutter-testing/ios/Runner.xcworkspace/contents.xcworkspacedata: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @override | ||
| void dispose() { | ||
| if (_adId != null) { | ||
| CloudX.destroyAd(adId: _adId!); |
There was a problem hiding this comment.
destroyAd() is async but not awaited. Ad may not be properly cleaned up before widget disposal.
There was a problem hiding this comment.
can't use await in dispose
| # cloudx_flutter: ^0.16.0 // latest prod | ||
| cloudx_flutter: # local | ||
| path: ../cloudx_flutter_sdk |
There was a problem hiding this comment.
I don't think we need that switched, it is for pubs, we won't be working on this flutter demo app much. Thinking that would be overkill.
Code Review: PR #15 - CloudX Flutter Testing AppThanks for this contribution! I've completed a comprehensive review. The testing app is functional and demonstrates CloudX SDK integration well, but there are several issues that need to be addressed before merging. 🚨 Critical Issues (Must Fix Before Merge)1. Package Name ConflictLocation: The package is named Fix Required:
2. Memory Leaks - Missing destroyAd() CallsLocation:
The dispose() methods only call
Impact: Memory leaks, orphaned auto-refresh timers, resource exhaustion in long-running test sessions. Fix Required: @override
void dispose() {
// Need to destroy the underlying ad instance
_controller.stopAutoRefresh(); // Stop timers
CloudX.destroyAd(_adId); // Clean up native resources
_controller.dispose();
_scrollController.dispose();
super.dispose();
}Note: This may reveal a gap in the SDK wrapper - widget-based ads may not expose a proper destroy mechanism via controllers. 3. Hardcoded Production App KeysLocation:
Production CloudX keys are committed to version control: static const String _defaultIosAppKey = 'uOPdnQC_zu0gJs8HP3cBs'; // Production!
static const String _defaultAndroidAppKey = '69TdNnN1EcNpeyWWkLhBS'; // Production!Security Risk: Keys can be scraped, no rotation possible, violates best practices. Fix Required: static const String _defaultIosAppKey = 'YOUR_IOS_APP_KEY_HERE';
static const String _defaultAndroidAppKey = 'YOUR_ANDROID_APP_KEY_HERE';Add documentation on where to obtain real keys for testing. 4. iOS Podfile Uses Non-Portable Local PathsLocation: pod 'CloudXCore', :path => '../../cloudx-ios-private/core/', :inhibit_warnings => true
pod 'CloudXMetaAdapter', :path => '../../cloudx-ios-private/adapter-meta/', :inhibit_warnings => trueImpact: Won't build for other developers, blocks CI/CD, prevents external contributions. Fix Required: Remove these local path overrides. The Flutter plugin dependency should pull the correct published SDK version from CocoaPods.
|
|
|
|
|
5-6: removed |
|
|
|
No description provided.