-
Notifications
You must be signed in to change notification settings - Fork 33
[SDK-18] - Deeplinking BCIT #979
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Implements Deeplinking to invoke app from outside and invoking sdk's handler methods
Ayyanchira
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial pass given. Tested locally and verified working. Now checking on CI
| private lateinit var tvHeader: TextView | ||
| private lateinit var tvUrl: TextView | ||
| private lateinit var tvPath: TextView | ||
| private lateinit var tvHost: TextView | ||
| private lateinit var tvScheme: TextView | ||
| private lateinit var tvStatus: TextView |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why there is a tv prefix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how autoverify doesnt check existence of assetlinks and could still open the app during testing. Perhaps not having other apps contesting for it is the reason
Deeplink.BCIT.on.Android.mov |
joaodordio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left one comment for us to discuss but looking good!
...ration-tests/src/main/java/com/iterable/integration/tests/activities/DeepLinkTestActivity.kt
Outdated
Show resolved
Hide resolved
|
Question about integration test These tests are thorough but I'm wondering about the cost/benefit. Integration tests are great for proving the system works end-to-end, but they're slow and harder to maintain than unit tests. Working in the flaky tests and everything made me even more skeptical of them. What if we:
Integration tests could fill gaps where we can't unit test yet (DI issues, Android framework dependencies). For pure logic, unit tests can be better. |
|
@franco-zalamena-iterable Those are some really great points. Infact the idea of integration test is to cover certain basic flow of the core functionality of what the SDK offers. I believe Deeplink covers a lot outside of SDK's scope which also gets tested although that use case is still not covered here. (example - a travel app website on chrome showing |
And totally agree this |
…/activities/DeepLinkTestActivity.kt This totally makes sense. Create a dictionary and make a json out of it? while you can make a json and fill it? 🤦♂️ Co-authored-by: Joao Dordio <joaodordio@icloud.com>
joaodordio
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
🔹 Jira Ticket(s) if any
✏️ Description