Skip to content
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

fix(ios)(8_0_X) : Added log functionality #10922

Merged
merged 5 commits into from Jun 3, 2019

Conversation

vijaysingh-axway
Copy link
Contributor

@build build added this to the 8.0.2 milestone May 29, 2019
@build build requested a review from a team May 29, 2019 22:16
@build
Copy link
Contributor

build commented May 29, 2019

Fails
🚫 Tests have failed, see below for more information.
Messages
📖

💾 Here's the generated SDK zipfile.

📖 ❌ 1 tests have failed There are 1 tests failing and 450 skipped out of 3025 total tests.

Tests:

Classname Name Time Error
android.emulator.Titanium.UI.WebView .zoomLevel 10.002 timeout of 10000ms exceeded

Generated by 🚫 dangerJS against bfc6f24

@@ -443,6 +443,9 @@ - (WKUserScript *)userScriptTitaniumInjectionForAppEvent
warn: function(message){ \
window.webkit.messageHandlers._Ti_.postMessage({name:'warn', method: 'log', callback: Ti._JSON({level:'warn', message:message},1)},'*'); \
}, \
log: function(level, message){ \
window.webkit.messageHandlers._Ti_.postMessage({name:'level', method: 'log', callback: Ti._JSON({level:'level', message:message},1)},'*'); \
Copy link
Contributor

Choose a reason for hiding this comment

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

You're passing in the string literal 'level' instead of the actual value of the level argument.

Also is there any way to add a test for this? Open a Ti.UI.WebView, have it call Ti.API.log('warn', 'message'); and verify it actually occurred somehow? (I'm guessing not, since it goes directly to our APIModule and we can't intercept it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Thanks!
I don't think we can write test case for same reason as you have mentioned. Probably we can add event listener in APIModule and fire some event when log api get called. For this we have to make some modification inside APIModule. I am not sure if it is right way.

@keerthi1032
Copy link
Contributor

FR Passed. logs shown as expected in console .
Test Environment:
Name = Mac OS X
Version = 10.13.6
Architecture = 64bit
Node.js
Node.js Version = 8.9.1
npm Version = 5.5.1
Titanium CLI
CLI Version = 5.1.1
Titanium SDK
SDK Version = local sdk 8.0.2.v20190530104542
CLi =7.0.11
Device =iPhone 6s ios12
Simulator =iphone xr ios12

@keerthi1032
Copy link
Contributor

Jenkin failed. Not able to merge PR. waiting for resolving jenkin's failure

@sgtcoolguy sgtcoolguy merged commit 6a9f4ea into tidev:8_0_X Jun 3, 2019
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.

None yet

4 participants