Improve tests and cleanup unnecessary code

.github/workflows/test.yml

@@ -32,7 +32,7 @@ jobs:
             gemfile: gemfiles/rails_6.0.sqlite3.gemfile
           - ruby: 2.6
             gemfile: gemfiles/rails_6.1.sqlite3.gemfile
-          - ruby: "3.0"
+          - ruby: "3.1"
             gemfile: gemfiles/rails_7.0.sqlite3.gemfile
 
     env:

CHANGELOG.md

@@ -1,7 +1,9 @@
 # Changelog
 
 * `Unreleased` - [View Diff](https://github.com/cortiz/prawn-rails/compare/v1.4.2...master)
-  - Nothing yet
+  - Remove unnecessary method `get_metadata` from `PrawnRailsHelper` which was getting included into `ActionView::Base`
+  - Remove unnecessary rescue in `PrawnRails.config`
+  - Remove redundant method `PrawnRails::Document.extensions`
 
 * `v1.4.2` - [View Diff](https://github.com/cortiz/prawn-rails/compare/v1.4.1...v1.4.2)
   - Remove dependency on `railsties` and instead only depend on `actionview`

Gemfile

@@ -1,5 +1,3 @@
 source "http://rubygems.org"
 
 gemspec
-
-gem 'appraisal'

README.md

@@ -103,6 +103,7 @@ prawn_document do |pdf|
   pdf.table @products.collect{|p| [p.name,p.price]}
 end
 ```
+
 #### Using Custom options
 
 ```ruby
@@ -113,6 +114,19 @@ prawn_document(page_layout: :landscape) do |pdf|
 end
 ```
 
+#### Without using the prawn_document helper
+
+```ruby
+doc = PrawnRails::Document.new(page_layout: :landscape)
+
+doc.text "Page 1"
+doc.start_new_page
+doc.text "Page 2"
+
+pdf_str = doc.render
+```
+
+
 # Credits
 
 Maintained by Weston Ganger - @westonganger

lib/prawn-rails/config.rb

@@ -5,13 +5,8 @@ module PrawnRails
 
   @config = OpenStruct.new(page_layout: :portrait, page_size: "A4", skip_page_creation: false)
 
-  def config
-    begin
-      block_given? ? yield(@config) : @config
-    rescue => e
-      puts e
-      puts e.backtrace
-    end
+  def config(&block)
+    block_given? ? yield(@config) : @config
   end
 
 end

lib/prawn-rails/document.rb

@@ -2,33 +2,19 @@
 require 'prawn/table'
 
 module PrawnRails
-
-  # This derives from Prawn::Document in order to override defaults. 
-  # Note that the Prawn::Document behaviour itself shouldn't be changed.
   class Document < Prawn::Document
-    def self.extensions
-      Prawn::Document.extensions
-    end
-
     def initialize(options = {})
-      if PrawnRails.config.respond_to?(:to_h)
-        options.reverse_merge!(PrawnRails.config.to_h)
-      else
-        options.reverse_merge!(PrawnRails.config.marshal_dump)
-      end
+      options = PrawnRails.config.marshal_dump.merge(options) ### For Ruby 1.9.3 support, use `marshal_dump` instead of `to_h`
 
       super(options)
     end
 
-    # Typically text expects a string. But Rails views have this interesting concept that they implicitly call `to_s` on all the variables before rendering. So, passing an integer to text fails:
-    #
-    # pdf.text 10       #=> fails because 10 is not a string
-    # pdf.text 10.to_s  #=> works
-    #
-    # To circumvent this situation, we call to_s on value, and delegate action to actual Prawn::Document.
     def text(value, options = {})
+      # Typically text expects a string. But Rails views have this interesting concept that they implicitly call `to_s` on all the variables before rendering. So, passing an integer to text fails:
+      # pdf.text 10       #=> fails because 10 is not a string
+      # pdf.text 10.to_s  #=> works
+      # To circumvent this situation, we call to_s on value, and delegate action to actual Prawn::Document
       super(value.to_s, options)
     end
   end
-
 end

lib/prawn-rails/rails_helper.rb

@@ -6,7 +6,13 @@ module RailsHelper
     def prawn_document(options={})
       @filename ||= options[:filename]
 
-      options.reverse_merge!(get_metadata)
+      metadata = {}
+
+      if @filename
+        metadata[:info] = { Title: @filename.sub(/\.pdf$/i, '') }
+      end
+
+      options.reverse_merge!(metadata)
 
       pdf = PrawnRails::Document.new(options)
 
@@ -24,15 +30,5 @@ def prawn_document(options={})
       return pdf.render
     end
 
-    def get_metadata
-      return {} unless @filename
-
-      {
-        info: {
-          Title: @filename.sub(/\.pdf$/i, '')
-        }
-      }
-    end
-
   end
 end

prawn-rails.gemspec

@@ -30,6 +30,11 @@ Gem::Specification.new do |s|
   s.add_development_dependency "rails"
   s.add_development_dependency 'sqlite3'
   s.add_development_dependency 'appraisal'
+  s.add_development_dependency 'pry'
+
+  if RUBY_VERSION.to_f >= 3.1
+    s.add_development_dependency 'matrix'
+  end
 
   if RUBY_VERSION.to_f >= 2.4
     s.add_development_dependency 'warning'

test/dummy_app/config/application.rb

@@ -8,6 +8,10 @@
 
 module Dummy
   class Application < Rails::Application
+    if Rails::VERSION::STRING.to_f >= 5.1
+      config.load_defaults Rails::VERSION::STRING.to_f
+    end
+
     # Settings in config/environments/* take precedence over those specified here.
     # Application configuration should go into files in config/initializers
     # -- all .rb files in that directory are automatically loaded.

test/integration/prawn_rails_test.rb → test/integration/e2e_test.rb

@@ -1,10 +1,6 @@
-### FOR RAILS TESTS
-
 require 'test_helper'
-require 'pdf/reader'
 
 class NavigationTest < ActionDispatch::IntegrationTest
-  fixtures :all
 
   def confirm_pdf_format(source)
     reader = PDF::Reader.new(StringIO.new(source))
@@ -15,38 +11,34 @@ def confirm_pdf_format(source)
     assert_not page_str.include?("<h1>Hello World!</h1>")
   end
 
-  test "Registers :pdf mime type" do
+  test "registers :pdf mime type" do
     assert Mime::Type.lookup_by_extension(:pdf)
   end
 
-  test "Registers :prawn template handler" do
+  test "registers :prawn template handler" do
     assert ActionView::Template::Handlers.extensions.include?(:prawn)
   end
 
-  test "Renders html action" do
+  test "renders html action" do
     get '/reports/sample'
     assert_response :success
     assert_match("<h1>Hello World!</h1>", @response.body)
   end
 
-  test "Renders pdf to string" do
-    if RUBY_VERSION.to_f >= 2.7
-      skip "Test failing, couldnt figure it out, PR wanted"
-    end
-
-    pdf_str = ApplicationController.new.render_to_string("reports/sample.pdf", locals: {:@items => []})
+  test "renders pdf to string" do
+    pdf_str = ApplicationController.render(template: "reports/sample", formats: [:pdf], assigns: {:items => []})
 
     confirm_pdf_format(pdf_str)
   end
 
-  test "Renders sample pdf action" do
+  test "renders sample pdf action" do
     get '/reports/sample', params: {format: :pdf}
     assert_response :success
 
     confirm_pdf_format(@response.body)
   end
 
-  test "Renders table pdf action using auto-required plugin Prawn-Table" do
+  test "renders table pdf action using auto-required plugin prawn-table" do
     get '/reports/table', params: {format: :pdf}
     assert_response :success
 
@@ -60,23 +52,23 @@ def confirm_pdf_format(source)
     assert_equal(page_str, "1 2 3\n\n4 5 6\n\n7 8 9")
   end
 
-  test "Sets file name from '@filename' when present" do
+  test "sets file name from '@filename' when present" do
     get '/reports/ivar_filename.pdf'
 
     disposition_header = @response.headers["Content-Disposition"]
     assert disposition_header.include?("attachment")
     assert disposition_header.include?("ivar-filename.pdf")
   end
 
-  test "Maintains existing 'Content-Disposition' header" do
+  test "maintains existing 'Content-Disposition' header" do
     get '/reports/custom_headers.pdf'
 
     disposition_header = @response.headers["Content-Disposition"]
     assert disposition_header.include?("attachment")
     assert disposition_header.include?("custom-headers.pdf")
   end
 
-  test "Respects the 'filename' option alone" do
+  test "respects the 'filename' option alone" do
     get '/reports/custom_filename.pdf'
 
     disposition_header = @response.headers["Content-Disposition"]
@@ -92,7 +84,7 @@ def confirm_pdf_format(source)
     assert_not disposition_header.include?("filename")
   end
 
-  test "Respects both options on 'prawn-document' together" do
+  test "respects both options on 'prawn-document' together" do
     get '/reports/custom.pdf'
 
     disposition_header = @response.headers["Content-Disposition"]

test/test_helper.rb

@@ -6,11 +6,22 @@
   Warning.ignore(
     %r{mail/parsers/address_lists_parser}, ### Hide mail gem warnings
   )
+
+  Warning.ignore(
+    %r{pdf/reader/font.* assigned but unused variable}, ### Hide pdf/reader gem warnings
+  )
 rescue LoadError
   # Do nothing
 end
 
+if RUBY_VERSION.to_f >= 2.7
+  require "matrix"
+end
+
 require File.expand_path("../dummy_app/config/environment.rb",  __FILE__)
 require "rails/test_help"
 
 Rails.backtrace_cleaner.remove_silencers!
+
+require 'pry'
+require 'pdf/reader'

test/unit/config_test.rb

@@ -0,0 +1,34 @@
+require 'test_helper'
+
+class ConfigTest < ActiveSupport::TestCase
+
+  def setup
+    @original_config = PrawnRails.config.to_h
+  end
+
+  def teardown
+    PrawnRails.config do |config|
+      config.to_h.keys.each do |key|
+        config.delete_field(key)
+      end
+
+      @original_config.to_h.each do |k,v|
+        config[k] = v
+      end
+    end
+  end
+
+  test "has a default config" do
+    assert_equal PrawnRails.config.to_h, {page_layout: :portrait, page_size: "A4", skip_page_creation: false}
+  end
+
+  test "config can be set with block syntax" do
+    PrawnRails.config do |config|
+      config.page_layout = :landscape
+      config.page_size = "A8"
+    end
+
+    assert_equal PrawnRails.config.to_h, {page_layout: :landscape, page_size: "A8", skip_page_creation: false}
+  end
+
+end