Use AFMotion instead of BubbleWrap for HTTP (redux)

[mwise]

Hello,

I came across @killion's PR at #23 and took a stab at incorporating @tkadauke's feedback from that PR into this one (this branch is actually branched off of killion's from #23).

There's a potentially big caveat to this PR, however. It seems that the rubygems version of afmotion breaks the motion-support callbacks functionality (or at the very least breaks the specs - I haven't tested the callbacks in the wild yet). Hence the added afmotion from github in the Gemfile. For the life of me, I can't figure out what is going on there, since the last commit in afmotion coincides with the bump of gem version as well as the release on github and rubygems. The gemspec doesn't provide for using a github repo, so I'm not exactly sure how to proceed there. That seems like a maintainer-level consideration, so @tkadauke if you know how you want to approach that, please let me know.

Other than that, there's a lot of trailing whitespace cleanup that is incidental to this PR. I have an editor plugin to kill that wherever I find it. This PR was my first experience with motion-resource, so please let me know if there's anything awry in it or if anything needs changing.

Cheers,

Mark

[tkadauke]

Hey Mark,

First of all, thank you for this PR. It looks like we're on the right track here. Do all specs pass? I'd check it myself, but my version of RubyMotion is expired and I will wait until I have to use it again professionally before I buy it.

Regarding the gemspec issue, maybe we can ask @clayallsopp when he is going to release the next version? Or maybe he is willing to do a prerelease?

As for the whitespace -- everyone on the planet seems to agree to kill trailing whitespace so I will accept that change in coding style. However, it makes reviewing the harder, especially when there is no other change in a file.

--Thomas

[mwise]

Yep, the specs pass. Regarding the gemspec issue, I think that going to the afmotion team is the way to go. I can reach out to them when I get home tonight or if you prefer to channel the communication that works, too.

Also, for the whitespace, I can take another stab at branching off of @killion's work with my whitespace killer disabled. It definitely clutters the commits but gets easier once you adopt the style and automate the killing of it. Be prepared for a few whitespace cleanup commits subsequent to the work on the AFMotion conversion :-) If you don't mind conflating it here, then we can perhaps just roll with this PR once we've figured out the afmotion gem version issue.

[tkadauke]

Awesome. Let's see if @clayallsopp joins the conversation to find out about the AFMotion release schedule.

Am 02.07.2014 um 16:03 schrieb Mark Wise notifications@github.com:

Yep, the specs pass. Regarding the gemspec issue, I think that going to the afmotion team is the way to go. I can reach out to them when I get home tonight or if you prefer to channel the communication that works, too.

Also, for the whitespace, I can take another stab at branching off of @killion's work with my whitespace killer disabled. It definitely clutters the commits but gets easier once you adopt the style and automate the killing of it. Be prepared for a few whitespace cleanup commits subsequent to the work on the AFMotion conversion :-) If you don't mind conflating it here, then we can perhaps just roll with this PR once we've figured out the afmotion gem version issue.

—
Reply to this email directly or view it on GitHub.

[clayallsopp]

What is the exact problem that 2.3.1 is causing?

[mwise]

@clayallsopp When running the motion-resource specs w/ afmotion added by add_dependency in the gemspec, the MotionSuport::Callbacks-related specs in https://github.com/tkadauke/motion-resource/blob/master/spec/motion-resource/callbacks_spec.rb were failing. Requiring afmotion via git in the Gemfile made the specs pass. I'm very new to the project, but I believe that those specs passed when motion-resource was built against BubbleWrap::HTTP.

It was failing on https://github.com/rubymotion/motion-support/blob/master/motion/callbacks.rb#L148 with a 'undefined method `[]' for nil:NilClass' that made it seem like chain.config was nil. I was working on a different computer last night so I don't have the specific failure message handy. I will grab the stacktrace once I get home tonight and post it here.

[clayallsopp]

I see in your history that you used gem "afmotion", "~> 2.1.0". Can you try using gem "afmotion", "~> 2.3.0"? That is equivalent to HEAD on master (and has been for ~2 weeks)

[mwise]

Yeah, I'm pretty sure I did try w/ ~> 2.3.0, but I will confirm that this
evening and hopefully post some more details and dig into it a bit more
then.

Mark Wise

On Wed, Jul 2, 2014 at 2:59 PM, Clay Allsopp notifications@github.com
wrote:

I see in your history that you used gem "afmotion", "> 2.1.0". Can you
try using gem "afmotion", "> 2.3.0"? That is equivalent to HEAD on
master (and has been for ~2 weeks)

—
Reply to this email directly or view it on GitHub
#24 (comment)
.

[mwise]

Ok, yeah it still bombs out w afmotion ~> 2.3.0 in the Gemfile. Here's the exact error it's throwing from line 148 in motion-support's callbacks.rb:

callbacks
  - should run create callbacks [ERROR: NoMethodError - undefined method `[]' for nil:NilClass]

NoMethodError: undefined method `[]' for nil:NilClass
        callbacks.rb:148:in `block in apply:': callbacks - should run create callbacks
        callbacks.rb:267:in `block in compile'
        callbacks.rb:308:in `block in __define_callbacks:'
        callbacks.rb:73:in `run_callbacks:'
        crud.rb:18:in `create:'
        spec.rb:316:in `block in run_spec_block'
        spec.rb:440:in `execute_block'
        spec.rb:316:in `run_spec_block'
        spec.rb:331:in `run'

1 specifications (0 requirements), 0 failures, 1 errors

I think I'm officially stumped on this one. I can reliably get the suite (specifically the callback stuff) to pass w/ gem "afmotion", :git => 'https://github.com/usepropeller/afmotion.git' in my Gemfile, regardless if I have s.add_dependency 'afmotion', "~> 2.3.1" or even ~> 2.2.0 in the motion-resource.gemspec or not. When I go off of the gemspec, it bombs out with the error above.

I have a suspicion that it is related to webstub, but it doesn't seem to matter whether I require that via git via the Gemfile or via the gemspec. Even when running just the callbacks_spec.rb, I'll get consistent failures until I add the git-powered afmotion line in Gemfile. stub_request is used a lot throughout the suite, and I'm not entirely convinced that there's not some bleed happening somewhere. There's probably a bit of refactoring that could be done to narrow down the potential side-effect cases.

@clayallsopp I believe you that the afmotion gem version is up-to-date with the github commits ending two weeks ago. It certainly seemed that way to me. Hence the stumping. So if you have any thoughts or suggestions, I'm all ears. My use-case for motion-resource doesn't require the callbacks stuff at all, so I may step away from it to retain my sanity.

Cheers,

Mark

[clayallsopp]

I checked out this branch locally and could not get any consistent behavior. Sometimes the tests would pass using an old version of AFMotion (I tried 2.2.0 and 2.1.5 with success), sometimes the :git version would fail. I tried targeting different AFMotion/AFNetworking version combinations but didn't find any patterns.

I have a feeling this is a RubyMotion-level problem, but unfortunately I can't get the project in a consistently reproducible state =\

[mwise]

I saw some inconsistent behavior, as well. I realize now I neglected to include that in my last post. I basically got to where I was changing the afmotion version, emptying the gemset, bundling, rake cleaning, then running the suite. The github-based afmotion gem seemed to perform better on the whole, but was still inconsistent and I suppose could have just been a red herring.

I had seen some posts on google groups re: webstub acting strange after failed spec runs and rake clean helping to resolve it. Unfortunately, I found that not to be the case - although the failing behavior did seem to "stick" a bit or at least occur in streaks. I looked into the webstub code and didn't notice anything obvious.

I'm not sure if you dug into the errors from motion-support, but one thing I noticed was that - when it failed - it seemed like it was looking for chain.config[:terminator] expecting it to be a Callbacks::CallbackChain, but it was actually a Callbacks::Callback. I think I was crosseyed by the time I got down to digging through the callbacks.rb code, but perhaps that will be a clue to someone more familiar w/ MotionSupport::Callbacks.

[Bodacious]

+1

Is this merge request going be merged in?