handle recent Rack v3 changes in tests & example #209
Conversation
| require "rack/lobster" | ||
| rescue LoadError | ||
| require "rack/session" | ||
| require "rackup" |
There was a problem hiding this comment.
on second thought, I'm not sure if this line is needed. @robbkidd ?
There was a problem hiding this comment.
🤔 I think you may be right. I will check!
There was a problem hiding this comment.
It wasn't ✨ Just pushed a commit removing the duplicate "rackup" and also adding the rack-session dep to the appraisals file (not just directly in the .gemfile for rack_3)
MikeGoldsmith
added
the
type: maintenance
emilyashley
added
status: oncall
There was a problem hiding this comment.
Nice work! I'm not super familiar with Ruby, so I'll add my notes here from trying things out with this PR:
These changes worked for running bundle exec appraisal rake test. This makes sense - it worked in Circle! But when I tried bundle install, it was failing with this error:
[!] There was an error parsing ``rack.gemfile``: No such file or directory @ rb_sysopen - /Users/jamiedanielson/code/beeline-ruby/gemfiles/rack.gemfile. Bundler cannot continue.
I believe this was actually because of the rack example app, which should maybe be updated with this change? In examples/rack/Gemfile I added gem "rackup"; in examples/rack/main.rb I added require "rackup" to the top, and updated the handler to handler = Rackup::Handler::WEBrick (previously Rack::).
Now I can run bundle install successfully, and from examples/rack/ I can run bundle exec ruby main.rb, which starts the server. Then I curl localhost:8080 to get aHello from Honeycomb reply, and see trace data in the console {"meta.beeline_version":"2.11.0","meta.local_hostname":"jamiedanielson","meta.instrumentations_count":10,"meta.instrumentations":"[\"active_support\"...
::Rack::VERSION was deprecated in v3.0[1]. Also, turns out, it's the Rack *protocol* version, not the version of the Rack gem. [1] rack/rack@856934f ::Rack.release is the gem version, has existed since at least v0.4,[2] and--bonus!--is already a dotted-string version number. [2] https://github.com/rack/rack/blob/rack-0.4/lib/rack.rb#L24-L27
With some commentary thrown in because I didn't know what the heck Lobster was before this blew up the nightly, an if-else makes explicit our expectation that one way works with older Rack and the other way works with newer Rack.
You're cute, but too much trouble. Co-authored-by: Jamie Danielson <[email protected]>
OKAY, copper. FINE.
emilyashley
changed the title
emilyashley
changed the title
| def call(_env) | ||
| [200, { "content-type" => "text/plain" }, ["Hello world!"]] | ||
| end | ||
| end |
There was a problem hiding this comment.
Huge fan of replacing Rack::Lobster—as cute at it is—with a simple test app of our own to reduce dependencies.
| require "warden" | ||
|
|
||
| class AnApp | ||
| def call(_env) |
There was a problem hiding this comment.
Starting the unused variable with an underscore will appease the Lint/UnusedMethodArgument Rubocop cop.
We don't use it for any assertions, only to create the session. Simplify for future humans reading these tests.
|
| end | ||
| let(:auth) { Authenticate.new(honeycomb) } | ||
| let(:warden) do | ||
| Warden::Manager.new(auth) do |manager| | ||
| manager.default_strategies :test | ||
| end | ||
| end | ||
| let(:session) { Rack::Session::Cookie.new(warden, secret: "honeycomb") } | ||
| let(:secret) { "honeycombsecretneedstobe64characterslongforrack3sonowitslongeryay" } |
robbkidd
changed the title
robbkidd
changed the title
Which problem is this PR solving?
Short description of the changes