search

jspargo / dependabot-core

🤖 The core logic behind Dependabot's update PR creation
https://dependabot.com
MIT License
0 stars 2 forks source link

Enable pending update_checker_spec tests #7

Closed jspargo closed 4 years ago

jspargo commented 4 years ago

Fixes #5

RuudPuts commented 4 years ago

We'll need to fix the branching I think :)

Currently there's add-cococapods and add-cococapods-RP. The latter contains all changes & fixes and is quite far ahead of add-cococapods. Should we merge add-cocoapods-RP into add-cocoapods? See the compare here; https://github.com/jspargo/dependabot-core/compare/add-cocoapods...jspargo:add-cocoapods-RP?expand=1

jspargo commented 4 years ago

@RuudPuts I merged add-cocoapods (where #2 was merged) back into add-cocoapods-RP (where #4 was merged), and that compare link above looks better now, but I think you're right that it's more complicated than it needs to be!

My thinking here is that I was to use add-cocoapods as the main branch for https://github.com/dependabot/dependabot-core/pull/731, but that we use add-cocoapods-RP (or another branch - master perhaps?) as a sort of forked version add-cocoapods. It's not a big deal, but I'd like to do as few squashed merges to add-cocoapods as possible just so the maintainers of dependabot-core don't get bombarded by CI runs and notifications until we want them to see them ;)

Shall we agree going forward to raise PRs into add-cocoapods-RP, then a big PR into add-cocapods once we have fixed all the issues?

RuudPuts commented 4 years ago

I think that's a good plan. Squashing all commits into some bigger updates for the main repo sounds good. I've double checked #4 and it went to add-cocoapods-RP so that all went well.

I'll probably get on the test tomorrow, hopefully we can get some nice results out of it!

RuudPuts commented 4 years ago

I've had some troubles reproducing the issues, as on my local machine the tests still run fine, but I think when I'm running them in a Docker container I'm looking at the same issues as you and on CircleCI. At least the tests hang when it reaches the it statements enabled in this PR. After spending a few hours I still have no idea what's going on.

What I did find is the following. When the CocoaPods analyzer is set to verbose mode, you can replace analyzer.config.silent=true with analyzer.config.verbose = true the container gets stuck on 

Finding Podfile changes
  - Alamofire
  - Nimble

Resolving dependencies of
  CDN: trunk Relative path: all_pods_versions_d_a_2.txt modified during this run! Returning local
  CDN: trunk Relative path: Specs/d/a/2/Alamofire/4.6.0/Alamofire.podspec.json exists! Returning
  local because checking is only perfomed in repo update
  CDN: trunk Relative path: all_pods_versions_d_c_d.txt modified during this run! Returning local

After hitting ctrl+c a bit more is logged:

RSpec is shutting down and will print the summary report... Interrupt again to force quit.
  CDN: trunk Relative path: Specs/d/c/d/Nimble/3.0.0/Nimble.podspec.json exists! Returning local
  because checking is only perfomed in repo update
  CDN: trunk Relative path: Specs/d/a/2/Alamofire/3.0.1/Alamofire.podspec.json exists! Returning

While on my local machine it continues

Finding Podfile changes
  - Alamofire
  - Nimble

Resolving dependencies of
  CDN: trunk Relative path: all_pods_versions_d_a_2.txt modified during this run! Returning local
  CDN: trunk Relative path: Specs/d/a/2/Alamofire/4.6.0/Alamofire.podspec.json exists! Returning local because checking is only
  perfomed in repo update
  CDN: trunk Relative path: all_pods_versions_d_c_d.txt modified during this run! Returning local
  CDN: trunk Relative path: Specs/d/c/d/Nimble/3.0.0/Nimble.podspec.json exists! Returning local because checking is only perfomed
  in repo update
  CDN: trunk Relative path: Specs/d/a/2/Alamofire/3.0.1/Alamofire.podspec.json exists! Returning local because checking is only
  perfomed in repo update
  CDN: trunk Relative path: Specs/d/c/d/Nimble/2.0.0/Nimble.podspec.json exists! Returning local because checking is only perfomed
  in repo update

Comparing resolved specification to the sandbox manifest
  A Alamofire
  A Nimble

What I have noticed is that the freeze is linked to the state of the trunk repo in ~/.cocoapods/repos/trunk. When I copy my local trunk to the container the tests run without hanging or failing. Deleting the folder on my local machine also makes the problem reproducible there. Strangely I did the same before #2 was merged and had no problems at that time. I've attached a ZIP file with the trunk which makes the tests succeed for me so you can try it and see if it makes the tests succeed on your machine as well. Download here

This all has given me an idea though. The test might become more reliable by reconstructing the trunk repo in ~/.cocoapods using the fixture files rather than stubbing all the requests, hoping it'll use the local files as cache. I'll give that a go tomorrow 🤞

RuudPuts commented 4 years ago

@jspargo I've figured out why the tests are freezing. While observing the ~/.cocoapods/repos/trunk directory I noticed a directory for the Nimble 2.0.0 podspec was created, but not filled. Adding the stub for it in dd78fcf fixed that and with it made the tests on the Docker container succeed! The run on CircleCI confirms this 🚀

The rest of the PR is cleanup to reduce duplicated code in update_checker_spec.rb and file_updater_spec.rb.