rubycdp / ferrum

Headless Chrome Ruby API
https://ferrum.rubycdp.com
MIT License
1.74k stars 123 forks source link

Explicitly require `pathname` #409

Closed ttilberg closed 11 months ago

ttilberg commented 1 year ago

Pathname moved from stdlib to default gems in 3.0, and isn't loaded by default.

https://rubyreferences.github.io/rubychanges/3.0.html#libraries-promoted-to-default-gems

Fixes #408

ttilberg commented 1 year ago

@route To be honest, I'm not totally certain which contexts require the require statement or not. Surprisingly, tests passed that should have raised while running on 3.2.2

Here's an example of trying to run it in a raw script, failing unless there's a require, as in #408

image

However, running tests succeeded, and I didn't see anything that should have helped with autoloading. I would have expected this specific test to fail:

~/dev/ferrum  408-pathname-is-default-gem ✔                                                                                                               2m
▶ be rspec spec/page/screenshot_spec.rb:118

Google Chrome 117.0.5938.92

Run options: include {:locations=>{"./spec/page/screenshot_spec.rb"=>[118]}}
Starting Puma
* Version 4.3.12 , codename: Mysterious Traveller
* Min threads: 0, max threads: 4
* Listening on tcp://127.0.0.1:50491

Ferrum::Page::Screenshot
  #screenshot
    supports screenshotting the page with a non-string path

Finished in 0.8508 seconds (files took 0.39272 seconds to load)
1 example, 0 failures

The other test that uses Pathname is currently specified to skip.

~/dev/ferrum  408-pathname-is-default-gem ✔                                                                                                               6m
▶ be rspec spec/node_spec.rb:728

Google Chrome 117.0.5938.92

Run options: include {:locations=>{"./spec/node_spec.rb"=>[728]}}
Starting Puma
* Version 4.3.12 , codename: Mysterious Traveller
* Min threads: 0, max threads: 4
* Listening on tcp://127.0.0.1:50701

Ferrum::Node
  #attach_file
    attaches a file when passed a Pathname (PENDING: No reason given)

Pending: (Failures listed here are expected and do not affect your suite's status)

  1) Ferrum::Node#attach_file attaches a file when passed a Pathname
     # No reason given
     # ./spec/node_spec.rb:728

Finished in 0.55437 seconds (files took 0.35922 seconds to load)
1 example, 0 failures, 1 pending

Not sure why require 'pathname' isn't required here, but is in other scripts.

route commented 11 months ago

@ttilberg maybe in tests there's a gem or dependency that requires "pathname". So thus in tests it works. Anyways it's better to require, and sorry for delays now I'm full time on Ferrum ;)