Added jackson libraries with a custom failing test #50

saruye · 2015-10-14T11:15:25Z

added jackson libraries with a failing test
test shows that jackson cannot demarshall embedded lists correctly

brunobowden · 2015-10-14T22:16:18Z

libraryBuilds/com.fasterxml.jackson.core/com.fasterxml.jackson.core-jackson-core/build.gradle

+
+autoConfigureDeps true
+    forceFilenameCollisionCheck false
+    translateArgs = ['--segmented-headers', '--extract-unsequenced', '--build-closure']


These are required flags to make the build work? If so, please add descriptions for each. You should also use groovy style syntax:

translateArgs '--segmented-headers' // Why this is needed? translateArgs '--extract-unsequenced' // Why this is needed? translateArgs '--build-closure' // Why this is needed?

@advayDev1 - is there any issue using --build-closure for the e2e test?

Please remove --build-closure from all of the files. If it doesn't build without --build-closure, that means we aren't translating the source jar correctly and/or you don't have all the dependencies listed.

brunobowden · 2015-10-14T22:30:05Z

@saruye - please address my comments and then we'll get @advayDev1 to have a look.

You can also add a comment or commit message: Fixes #48. That'll automatically close that issue when the PR is merged.

Finally in regards to the Jackson bug. Where is that tracked? It sounds like this is a J2ObjC issue rather than the plugin. Is it tracked already?

advayDev1 · 2015-10-14T22:35:15Z

...n-gson/com.google.code.gson-gson/src/test/java/com/google/code/gson/custom/GenericsTest.java

+/**
+ * Created by arne on 13.10.15.
+ */
+public class GenericsTest {


Why does Gson need to be modified?

advayDev1 · 2015-10-14T23:20:04Z

@saruye , also please follow the directions in:
https://github.com/j2objc-contrib/j2objc-common-libs-e2e-test/blob/f851db9eadfb0a649f6d8e09f5a1f7e7052c67c5/CONTRIBUTING.md#conditions-for-accepting-pull-requests
and
https://github.com/j2objc-contrib/j2objc-common-libs-e2e-test/blob/f851db9eadfb0a649f6d8e09f5a1f7e7052c67c5/CONTRIBUTING.md#preparing-your-pull-request-for-submission (rebasing / squashing / etc.)

saruye · 2015-10-15T10:40:04Z

@brunobowden I tried to address all your comments. If I missed something please let me know. I didn't add comments for the translate flags since I couldn't technically explain why they are needed (reduced them to one though). I just know that it doesn't compile without the flag. There might be an import cycle if I understand the flag correctly. I also didn't add the todo because I wasn't sure what to put there since it is mainly a j2objc issue like you said. Should I add something like:
TODO fix j2objc ticket and remove custom test
Not sure how you want to handle the extra test. I don't think it is necessary here once the build succeeds.
I created a ticket for this bug: google/j2objc#639

I just saw that I missed your comment with the commit message. I'll add this after your next feedback!

@advayDev1 I removed the gson code. I added it to see if there was a general limit with generic lists or if this was a specific jackson problem. I left it there because I thought it could help finding the problem.
The test is my own. I moved it to a different package now. It's there to help with the ticket mentioned above. I can remove it after this is fixed.

advayDev1 · 2015-10-15T20:55:45Z

...uilds/com.fasterxml.jackson.core/com.fasterxml.jackson.core-jackson-annotations/build.gradle

+}
+
+j2objcConfig {
+    // Almost always there are no tests provided in an external source jar.


I'm worried that you don't have autoConfigureDeps true in here.

advayDev1 · 2015-10-15T21:08:15Z

thanks @saruye , i've provided more comments.

advayDev1 · 2015-10-15T21:14:27Z

also @saruye - you may want to fix your github user config and/or local git config.
Your commit certification has your gmail email.
Your commit itself was authored by "[email protected]" - https://github.com/saruye/j2objc-common-libs-e2e-test/commit/19a6ccb7712645948b218d4c158c967bef4a4ca3.patch
And your commit was committed by something called '[email protected]' -

If all of these accounts are actually you, you should add the appropriate emails at:
https://github.com/settings/emails
Then GitHub can actually show you as the author.
If you did it right, @saruye should show up as the sole author on this page:
19a6ccb

saruye · 2015-10-19T08:01:39Z

@advayDev1 I worked on your comments. The custom-test was a try to work around the .gitignore and maybe separate between my tests and tests from the developers. I forgot to remove it after I gave up on this approach.
I will fix the commiter email when I prepare my PR for merging.
I saw that I added sourceCompatibility and the test config to the common/build.gradle. Should I revert that?

saruye · 2015-10-19T16:46:31Z

@advayDev1 in the latest travis build (https://travis-ci.org/j2objc-contrib/j2objc-common-libs-e2e-test/jobs/86138522) all tests did run through. Is it possible that this is because of the plugin update? Locally it is still failing, but I cannot upgrade to xcode 7 yet on my test machine.

advayDev1 · 2015-10-19T21:20:06Z

I have not updated the plugin in a while - it could not be that :)

advayDev1 · 2015-10-19T21:27:09Z

@saruye - so I'm comparing your failed run to the successful one. You've definitely changed the test itself.

On the failure (https://travis-ci.org/j2objc-contrib/j2objc-common-libs-e2e-test/jobs/85500882#L3685) your code was (0b5e511#diff-6b7fe9529973ed103aa4f8d7c95558c3R179). The stack trace is:

    at 0x0000000100b30ed3 org.junit.Assert.fail() + 16
    at 0x0000000100b30e72 org.junit.Assert.assertFalse() + 0
    at 0x00000001008e927a com.github.j2objccontrib.j2objcgradle.failing.GenericsTest.testDemarshallingListField() + 266
    at 0x0000000100d336fc java.lang.reflect.Method.invoke:object:() + 231

Locally is your failure the same?

saruye · 2015-10-20T21:57:22Z

I saw your build on travis with an update, but I guess that was not merged yet. And since the weekend I had trouble building locally after I merged some changes from master.
I now updated to xcode 7 and was able to reproduce the test running through. I also tested with the reversed test and it still fails. So I am only more confused why my changes did affect the test, since I only tried to break it down to the essentials.

advayDev1 · 2015-10-20T22:17:53Z

Paring down the test to only that which is necessary is the right thing to do. I cannot tell from the j2objcTest failure which line in the test is failing though.

saruye · 2015-11-12T17:37:32Z

The j2objc problem seems to be fixed:) (google/j2objc#639) I'll do some more cleanup of my PR on the weekend. And then I think it can be merged as soon as a new j2objc build is available and the plugin is updated to use it. What should I do with my test? Should I leave it for now or should I remove it?

brunobowden · 2015-11-12T17:40:18Z

With the e2e test? Please keep it around. We want to get this submitted once J2ObjC is updated and it's working well. Just ping us again when it's all working and ready to review. Thanks for your help.

advayDev1 · 2015-11-12T23:23:31Z

I think it would be helpful actually to submit the tests (after you clean them up) before the next j2objc release comes out, even knowing that it doesn't work in 0.9.8.2.1.

When the next j2objc release comes about, we'll bump the version in our config and it should 'just' work.

superarne · 2015-11-16T08:23:17Z

@brunobowden I meant my own unit test. I was just asking since no other library had such a test and because it is actually a test for j2objc and not for the gradle-plugin. So I wasn't sure if you wanted to keep it here.
Will clean it up and let you know when I think to be finished.

advayDev1 · 2015-11-16T19:00:45Z

nearly all the testing done in this repo is actually a test of j2objc and not the gradle plugin. that's perfectly ok, as our goal is to ensure that end-to-end, these libraries work (whether the issue is in the plugin, j2objc, or the library itself).

saruye · 2015-12-06T18:33:30Z

hi. sorry it took so long. I think I am through with my cleanup. Can you do another review?

brunobowden · 2015-12-09T00:08:31Z

.travis.yml

+  allow_failures:
+      # Blocked on (github issue url)
+#     - env: TEST_DIR=com.example-library
+      # Blocked on: https://github.com/j2objc-contrib/j2objc-common-libs-e2e-test/issues/48


Can you update the bug with the remaining issue and see if someone on there has insights on how to fix it.

Actually once this is submitted. Issue 48 should be closed as it now has a continuous build. You should have a separate bug explaining the build failure and then reference that on line 23 instead.

brunobowden · 2015-12-09T00:27:30Z

Hi @saruye, I think we're close to getting this in.

saruye · 2015-12-13T12:43:55Z

The j2objc issue is fixed and I tested it locally by using the master branch to build this repository. So once you update the e2e tests to use a version of j2objc where the issue is fixed everything should work. I thought I don't put this in the code so that you don't have to remove this comment once you are able to update the version. I added/updated a comment in both the travis and the test file.
I also renamed the test to something more meaningful. The test itself contains mainly code that "proved" the bug and not much else though.
Let me know if there is anything else.

I, Arne Osthues ([email protected], https://github.com/saruye), certify that a) this Contribution is my original work, and b) I have the right to submit this Contribution under the Apache License, Version 2.0 (the "License") available at http://www.apache.org/licenses/LICENSE-2.0, and c) I am submitting this Contribution under the License.

saruye force-pushed the feature/jackson branch from 483c8f9 to 373da6f Compare October 14, 2015 11:29

saruye mentioned this pull request Oct 14, 2015

com.fasterxml.jackson.core:jackson-core:2.6.2 - J2ObjC continuous build #48

Open

saruye force-pushed the feature/jackson branch 2 times, most recently from a0e3617 to 087f6e4 Compare October 14, 2015 12:26

brunobowden reviewed Oct 14, 2015
View reviewed changes

advayDev1 reviewed Oct 14, 2015
View reviewed changes

saruye force-pushed the feature/jackson branch from 5786837 to 19a6ccb Compare October 15, 2015 10:15

saruye mentioned this pull request Oct 15, 2015

Jackson deserialization failed when using Generic Types in an embedded list google/j2objc#639

Closed

advayDev1 reviewed Oct 15, 2015
View reviewed changes

saruye force-pushed the feature/jackson branch from d9d1985 to 30da242 Compare November 12, 2015 17:20

saruye force-pushed the feature/jackson branch 2 times, most recently from d469886 to b97261b Compare December 6, 2015 16:47

brunobowden reviewed Dec 9, 2015
View reviewed changes

saruye force-pushed the feature/jackson branch 2 times, most recently from dd517ce to 513236f Compare December 13, 2015 12:30

saruye force-pushed the feature/jackson branch from 513236f to c9e9ef2 Compare December 14, 2015 08:39

karussell mentioned this pull request May 8, 2017

Annotation type and annotation's creation method (create_XXXX) are missing google/j2objc#815

Open

Added jackson libraries with a custom failing test #50

Are you sure you want to change the base?

Added jackson libraries with a custom failing test #50

Uh oh!

Conversation

saruye commented Oct 14, 2015

Uh oh!

brunobowden Oct 14, 2015

Choose a reason for hiding this comment

Uh oh!

advayDev1 Oct 14, 2015

Choose a reason for hiding this comment

Uh oh!

brunobowden commented Oct 14, 2015

Uh oh!

advayDev1 Oct 14, 2015

Choose a reason for hiding this comment

Uh oh!

advayDev1 commented Oct 14, 2015

Uh oh!

saruye commented Oct 15, 2015

Uh oh!

advayDev1 Oct 15, 2015

Choose a reason for hiding this comment

Uh oh!

advayDev1 commented Oct 15, 2015

Uh oh!

advayDev1 commented Oct 15, 2015

Uh oh!

saruye commented Oct 19, 2015

Uh oh!

saruye commented Oct 19, 2015

Uh oh!

advayDev1 commented Oct 19, 2015

Uh oh!

advayDev1 commented Oct 19, 2015

Uh oh!

saruye commented Oct 20, 2015

Uh oh!

advayDev1 commented Oct 20, 2015

Uh oh!

saruye commented Nov 12, 2015

Uh oh!

brunobowden commented Nov 12, 2015

Uh oh!

advayDev1 commented Nov 12, 2015

Uh oh!

superarne commented Nov 16, 2015

Uh oh!

advayDev1 commented Nov 16, 2015

Uh oh!

saruye commented Dec 6, 2015

Uh oh!

brunobowden Dec 9, 2015

Choose a reason for hiding this comment

Uh oh!

brunobowden Dec 9, 2015

Choose a reason for hiding this comment

Uh oh!

brunobowden commented Dec 9, 2015

Uh oh!

saruye commented Dec 13, 2015

Uh oh!

Uh oh!