[2.0] Update the specs to pass under Bundler 2 by segiddins · Pull Request #5800 · rubygems/bundler

segiddins · 2017-06-20T22:17:31Z

What was the end-user problem that led to this PR?

The problem was we have all these amazing Bundler 2.0 features hidden behind feature flags. But we weren't testing all of bundler in that 2.0 mode.

Was was your diagnosis of the problem?

My diagnosis was we needed to get the bundler 2 specs running, and passing!

What is your fix for the problem, implemented in this PR?

My fix is to add a travis build entry to change version.rb to a 2.0 version and run the tests!

Why did you choose this fix out of the possible options?

I chose this fix because it will completely imitate what happens once we change the version on master, and by keeping the test suite passing on both 1.0 and 2.0 modes, we'll be in a position to release a 1.16 to which we'll be able to (relatively easily) backport fixes that land after master switches to completely target 2.0.

bundlerbot · 2017-06-22T00:19:37Z

☔ The latest upstream changes (presumably #5804) made this pull request unmergeable. Please resolve the merge conflicts.

segiddins · 2017-06-22T17:04:08Z

If this now passes, I'll restore all the other build matrix entries. Please review?

segiddins · 2017-06-22T21:31:11Z

Only thing failing on 2.0 are the real world specs 🎆

segiddins · 2017-06-22T22:35:02Z

Blocked by #5788

bundlerbot · 2017-06-23T06:08:01Z

☔ The latest upstream changes (presumably #5806) made this pull request unmergeable. Please resolve the merge conflicts.

jules2689 · 2017-06-23T17:15:39Z

.travis.yml

-  - 2.3.4
-  - 2.2.7
-  - 2.1.10
-  - 2.0.0


This is the version that ships with Mac, would it make sense to keep this right now?

Please see the PR description

Don't worry, I'll rebase out the deletion of all the build entries once this is ready to merge.

jules2689 · 2017-06-23T17:30:22Z

lib/bundler/definition.rb


-      @dependencies    = dependencies
+      @dependencies = dependencies
+      @dependencies_by_name = dependencies.group_by(&:name)


I'm a little concerned if this would introduce a boot time regression. I can take a look in a bit and test it out

Agree it's not ideal, but I can't think of how else to fix the only_update_to_newer_versions bug

I guess I could move this to additional_base_requirements_for_resolve so that we don't hit it in Bundler.setup ?

jules2689 · 2017-06-23T17:31:21Z

lib/bundler/definition.rb

-        dep = Gem::Dependency.new(locked_spec.name, ">= #{locked_spec.version}")
-        requirements[locked_spec.name] = DepProxy.new(dep, locked_spec.platform)
+        name = locked_spec.name
+        next requirements if @locked_deps[name] != @dependencies_by_name[name]


next statements are my favourite statements, only superseded by break since these mean less work ;P

but break here would have entirely different semantics, as this is a reduce

@segiddins I was just making a statement that next statements are one of my favourite because they mean less work == often more boot time perf. Was a stupid joke :)

andremedeiros · 2017-06-23T18:25:24Z

lib/bundler/definition.rb

      @unlocking = unlock == true || !unlock.empty?

-      @dependencies    = dependencies
+      @dependencies = dependencies


Can you fix the alignment here?

ugh yeah, rubocop should've caught that :/ (came from reverting another change)

andremedeiros · 2017-06-23T18:26:15Z

lib/bundler/source/git.rb

+      rescue GitError => e
        raise unless Bundler.feature_flag.allow_offline_install?
-        Bundler.ui.warn "Using cached git data because of network errors"
+        Bundler.ui.warn "Using cached git data because of network errors\n#{e}"


Maybe add a colon after errors to suggest the exception error contains details.

andremedeiros · 2017-06-23T18:28:09Z

spec/support/helpers.rb

-        @out = Thread.new { stdout.read }.value.strip
-        @err = Thread.new { stderr.read }.value.strip
+        command_execution.exitstatus = wait_thr && wait_thr.value.exitstatus
+        command_execution.stdout = Thread.new { stdout.read }.value.strip


Don't we want to .join the threads here?

value joins the thread

andremedeiros · 2017-06-23T18:31:23Z

@bundlerbot r+

bundlerbot · 2017-06-23T18:31:24Z

📌 Commit 63359d7 has been approved by andremedeiros

bundlerbot · 2017-06-23T18:31:28Z

⌛ Testing commit 63359d7 with merge 8558fac...

[2.0] Update the specs to pass under Bundler 2 I'm opening this up as a WIP because I want travis to run. Don't worry, I'll rebase out the deletion of all the build entries once this is ready to merge.

segiddins · 2017-06-23T18:33:43Z

@andremedeiros I haven't addressed your comments yet... (It's also failing on 1.8.7 because it doesn't seem to support \h)

bundlerbot · 2017-06-23T18:34:07Z

💔 Test failed - status-travis

andremedeiros · 2017-06-23T18:34:21Z

@segiddins my bad. Sorry 😢

…git data

…ore that example is run

segiddins · 2017-06-23T18:41:09Z

Rebased, added a PR description.

segiddins · 2017-06-23T18:53:19Z

No worries, just paranoid about pegging our travis VMs (earlier last week stuff was taking half a day to run)

indirect

lgtm too 👍🏻

segiddins · 2017-06-23T20:04:14Z

Its. 🍏. 🎆

@bundlerbot r+

bundlerbot · 2017-06-23T20:04:15Z

📌 Commit 717d194 has been approved by segiddins

bundlerbot · 2017-06-23T20:44:58Z

⌛ Testing commit 717d194 with merge a14b5c0...

[2.0] Update the specs to pass under Bundler 2 ### What was the end-user problem that led to this PR? The problem was we have all these _amazing_ Bundler 2.0 features hidden behind feature flags. But we weren't testing all of bundler in that 2.0 mode. ### Was was your diagnosis of the problem? My diagnosis was we needed to get the bundler 2 specs running, and passing! ### What is your fix for the problem, implemented in this PR? My fix is to add a travis build entry to change `version.rb` to a 2.0 version and run the tests! ### Why did you choose this fix out of the possible options? I chose this fix because it will completely imitate what happens once we change the version on `master`, and by keeping the test suite passing on both 1.0 and 2.0 modes, we'll be in a position to release a 1.16 to which we'll be able to (relatively easily) backport fixes that land after master switches to completely target 2.0.

bundlerbot · 2017-06-23T22:19:23Z

☀️ Test successful - status-travis
Approved by: segiddins
Pushing a14b5c0 to master...

segiddins force-pushed the seg-bundler-2-specs branch 2 times, most recently from 17a1ebc to a666ae4 Compare June 21, 2017 22:43

segiddins force-pushed the seg-bundler-2-specs branch 2 times, most recently from be4d8c9 to df105d8 Compare June 22, 2017 17:03

segiddins added 4 commits June 23, 2017 11:57

Introduce a CommandExecution helper class in the specs

20c89c6

Allow filtering specs based on the bundler version

a77ec22

[Rakefile] Allow stubbing the Bundler version on travis

188e794

[Travis] Begin testing Bundler 2

1342d93

segiddins force-pushed the seg-bundler-2-specs branch from df105d8 to fe6e03e Compare June 23, 2017 17:14

jules2689 reviewed Jun 23, 2017

View reviewed changes

segiddins added 7 commits June 23, 2017 12:43

Update the specs to pass under Bundler 2

07573bf

[UI::Shell] Warn on STDERR in Bundler 2

8737019

Get the specs passing in 1.0 mode

de77559

Filter git credentials when a revision is missing

383da3e

[Definition] Fully unlock on 2.0 when the dep in the Gemfile changes

47256d2

[Init] Don’t create a gems.rb when a Gemfile exists

864fe93

Get the 2.0 specs passing under 2.0

9e7a22b

segiddins force-pushed the seg-bundler-2-specs branch from fe6e03e to ca4b6f6 Compare June 23, 2017 17:44

segiddins changed the title ~~[WIP] Update the specs to pass under Bundler 2~~ [2.0] Update the specs to pass under Bundler 2 Jun 23, 2017

andremedeiros approved these changes Jun 23, 2017

View reviewed changes

segiddins added 5 commits June 23, 2017 13:36

[Source::Git] Print the underlying error when falling back to cached …

d3ac938

…git data

Improve realworld specs on 2.0

ab5ee13

Update the update spec for Bundler 2 allowing Bundler conflicts

3432b27

Update the version spec to handle when the specs start in the day bef…

217ecd5

…ore that example is run

[Definition] Avoid a group_by when not running a new resolve

717d194

segiddins requested review from colby-swandale and indirect June 23, 2017 18:38

segiddins force-pushed the seg-bundler-2-specs branch from 63359d7 to 717d194 Compare June 23, 2017 18:41

indirect approved these changes Jun 23, 2017

View reviewed changes

bundlerbot merged commit 717d194 into master Jun 23, 2017

This was referenced Jun 23, 2017

WIP: Setup Appveyor configuration #4299

Closed

No longer automatically subtract 1 from user-specified --jobs #5808

Closed

segiddins deleted the seg-bundler-2-specs branch June 23, 2017 22:34

Uh oh!

Conversation

segiddins commented Jun 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What was the end-user problem that led to this PR?

Was was your diagnosis of the problem?

What is your fix for the problem, implemented in this PR?

Why did you choose this fix out of the possible options?

Uh oh!

bundlerbot commented Jun 22, 2017

Uh oh!

segiddins commented Jun 22, 2017

Uh oh!

segiddins commented Jun 22, 2017

Uh oh!

segiddins commented Jun 22, 2017

Uh oh!

bundlerbot commented Jun 23, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andremedeiros commented Jun 23, 2017

Uh oh!

bundlerbot commented Jun 23, 2017

Uh oh!

bundlerbot commented Jun 23, 2017

Uh oh!

segiddins commented Jun 23, 2017

Uh oh!

bundlerbot commented Jun 23, 2017

Uh oh!

andremedeiros commented Jun 23, 2017

Uh oh!

segiddins commented Jun 23, 2017

Uh oh!

segiddins commented Jun 23, 2017

Uh oh!

indirect left a comment

Choose a reason for hiding this comment

Uh oh!

segiddins commented Jun 23, 2017

Uh oh!

bundlerbot commented Jun 23, 2017

Uh oh!

bundlerbot commented Jun 23, 2017

Uh oh!

bundlerbot commented Jun 23, 2017

Uh oh!

Reviewers

Assignees

segiddins commented Jun 20, 2017 •

edited

Loading