-
Notifications
You must be signed in to change notification settings - Fork 35
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
[JENKINS-58490] Support symbol files #37
[JENKINS-58490] Support symbol files #37
Conversation
76d0ded
to
a73d1fa
Compare
Is there a chance this (or the other one) will be committed and released before the HockeyApp execution date? Was hoping to be able to switch without (effectively) losing exception handling support, as that is mostly useless without the symbol files for iOS. Also, not sure what the UI will be, but I would make the label "Apple dSYM" or something that mentions dsyms, as that is the name most would be familiar with. |
@carllindberg That's the aim. However, I can't guarantee anything at this stage. As for the UI it will be |
OK anyone watching this is the pretty much how I see the symbols upload working. It should support both IOS and Android. However I haven't yet had time to a) write any unit tests or b) test this. I'll do that in a bit, but feel free to test this branch yourselves. It seems like there's a lot of demand for this so I'd be happy to release this without the unit tests, but I do want to at least get some feedback from real users on both Android and IOS platforms. I can do Android, but I don't have a way of testing IOS so would appreciate that help. |
If you don't want to compile yourself then the you can grab the latest build from this branch here. |
I will give it a try today @mezpahlan |
I've tested this with an Android app and it works well! I'm going to continue to write unit tests but will wait until I get an all clear from an IOS user. |
This field is optional and should not error if you do not supply it.
Sorry, wasn't able to find the time on Friday. Will do the iOS test today hopefully. |
As I am working on another machine today and got again stuck with the autogeneration not kicking in (not sure what's wrong with my setup), I have used the hpi from the PR to test. What I found weird is that you get a red warning if you do not provide a symbols path. At it is optional, I would remove that warning, otherwise it could be a bit confusing to the user. Besides that, from the logs it seem to have worked, but I do not see the Symbols: File found matching pattern: build/xxx.ipa I do not think that I will have time today to look into the code, but I will try tomorrow morning. |
Thanks @Kilomoana83. I appreciate the help. I noticed that too and have made a few changes to that screen whilst trying to write unit tests to the PR. I think in the interest of time I'll do the unit tests in a separate PR soon after this but will incorporate any changes that mention from your tests tomorrow including the one above. |
It worked for me as well. Uploaded, and I saw symbolicated backtraces in AppCenter with crashes. Thanks so, so much. I will second Kilomoana83's point that it complains if the dSYM path is not supplied, when it should be optional. (It's possible to use AppCenter without using its crash-catching stuff.) Also, would probably be better to have the debug symbols path be right next to the app path in the Jenkins UI, not at the bottom (the values can be related or at least very similar). My question on the label earlier was your SymbolType enum internally, but it appears that value does not get shown to the user, so that was moot. Thanks again. |
Hello,
Not sure where I should look at first to find the cause, any help would be greatly appreciated. |
Hi @michaljrk The exception is:
We basically check the file extension to determine whether you're uploading an IOS app or an Android app. Really simply: Does that help? |
Hi @Kilomoana83 did you get a chance to test this? I'd like to merge this if possible but wanted to wait for your OK first. |
Separate the library that does the parsing from the rest of the code so that we can mock it more easily.
@Kilomoana83 if you're looking for where the mappings / symbols files are located via the AppCenter UI then I agree it isn't in the most obvious place but I have found mine under Diagnostics -> Mappings for my Android project. Maybe for IOS it is the same? |
@carllindberg thanks for testing this. To answer your points:
This should be fixed now.
I have no preference on that. At the moment I want to fix a couple of more serious bugs before looking at the UI side of things which I am sure can be improved in line with your suggestion. If you don't mind could you raise a ticket for that in JIRA (instructions in the README) and I'll look at it when I have time (or someone else can once I've agreed the acceptance criteria). There's a few other higher priority issues that I want to resolve first though so if you can live with it at the bottom for now that might have to do for now. And if I don't get a reply back by tomorrow from the original requester I'll go ahead and merge this on the strength of your test. |
Thanks for your reply @mezpahlan . That's the reason probably as I'm trying to upload macOS app using zipped format ( |
Hi @michaljrk Oh I see. We don't officially support MacOS apps just yet. I think we probably should so could you raise a ticket in JIRA (instructions in the README) and I (or someone else) can look at how to add support. |
Issue created: Thanks a lot! |
@mezpahlan hey, yes, sorry forgot to update. The displaying in appcenter could be better and is only displaying the symbols when there is a crash. The mapping view from Android does not seem to exist. |
Thanks @Kilomoana83 I think at this stage it is now an issue with how AppCenter displays the symbol files for IOS only upon a crash. My reading of the IOS Symbolication page in the AppCenter documentation leads me to believe that this is as expected. I'm happy to look into things again under a different JIRA issue in the future if things still aren't working but for now I think uploading of the symbols files is working for Android (based on my observations) and IOS (based on @carllindberg's observations). I'll go ahead and merge this now. Many thanks for persisting with this. And sorry it took so long since you originally raised it. |
Builds upon the good work from #30 by @Kilomoana83 . Supports symbol upload functionality. This is an alternative implementation based on the PR comments I left in the pull request. We will only merge one of them.
The main difference here is that this PR aims to support both IOS and Android symbol files and uses a different architecture of the plugin that is more in line with what existed.