Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Method argument replaced by an instance of Minitest::Expectation #521

Closed
ammar opened this issue Apr 17, 2015 · 16 comments
Closed

Method argument replaced by an instance of Minitest::Expectation #521

ammar opened this issue Apr 17, 2015 · 16 comments

Comments

@ammar
Copy link

ammar commented Apr 17, 2015

Encountered after upgrading to minitest 5.6.0.

The following NameError occurs because a method argument is being replaced by an instance of Minitest::Expectation.

  1) Error:
InvoicePDFTest#test_0001_generates a document:
NameError: no member 'bold' in struct
    lib/syntax/pdf/style.rb:82:in `[]'
    lib/syntax/pdf/style.rb:82:in `style_span'
    lib/syntax/pdf/table.rb:95:in `value_list_row'
    lib/syntax/pdf/table.rb:119:in `block in value_list_table'
    lib/syntax/pdf/table.rb:119:in `map'
    lib/syntax/pdf/table.rb:119:in `value_list_table'
    app/pdfs/invoice_pdf.rb:76:in `invoice_info'
    app/pdfs/invoice_pdf.rb:87:in `block in invoice_details'
    lib/syntax/pdf/layout.rb:61:in `block (2 levels) in split_box'
    lib/syntax/pdf/layout.rb:60:in `block in split_box'
    lib/syntax/pdf/layout.rb:37:in `split_box'
    app/pdfs/invoice_pdf.rb:82:in `invoice_details'
    app/pdfs/invoice_pdf.rb:227:in `timesheet_invoice'
    app/pdfs/invoice_pdf.rb:13:in `block in initialize'
    lib/syntax/pdf/document.rb:74:in `render_document'
    app/pdfs/invoice_pdf.rb:13:in `initialize'
    test/pdfs/invoice_pdf_test.rb:5:in `new'
    test/pdfs/invoice_pdf_test.rb:5:in `setup'

The argument in this case is the options in the following:

  79       def style_span(str, options = {}, endl = false)
  80         out = str.dup
  81
  82         if options[:bold]
  83           out = style_tag(out, :b)
  84         end
  85
  86         if options[:italic]
  87           out = style_tag(out, :i)
  88         end
  89
  90         if options[:size]
  91           out = style_tag(out, :font, size: options[:size])
  92         end
  93
  94         if options[:color]
  95           out = style_tag(out, :color, rgb: options[:color])
  96         end
  97
  98         case options[:font]
  99           when Hash
 100             out = style_tag(out, :font, options[:font])
 101           when Numeric
 102             out = style_tag(out, :font, size: options[:font])
 103         end
 104
 105         out << "\n" if endl
 106
 107         out
 108       end

The Minitest::Expectation that is replacing the argument is:

 #<struct Minitest::Expectation target=nil, ctx=#<Syntax::PDF::Stylesheet label={:size=>10, :italic=>true, :color=>"444444"}, value={:size=>10, :bold=>true}>>

The expected argument is:

   {:size=>10, :italic=>true, :color=>"444444"}

A gist with the tests in question and test_helper.rb can be found here

@ammar
Copy link
Author

ammar commented Apr 17, 2015

There are no direct uses of _ in the call stack. Of the gems used by the app, i18n-0.7.0 and binding_of_caller-0.7.2 define a method with that name, but neither of them is used, directly or indirectly as far as I can tell.

This specific test file uses assertion style, but there are other files in the suite that use the expectation style.

@zenspider
Copy link
Collaborator

OK. Good. Since you're doing assertion-style, please re-run your tests with MT_NO_ISOLATE=1. Something like:

rake MT_NO_ISOLATE=1 TESTOPTS="-n /generates a document/"

if that works, re-run with MT_NO_ISOLATE again, but without the TESTOPTS, to un-eliminate cross-test contamination.

@daxadax
Copy link

daxadax commented Apr 17, 2015

I've got a similar issue. I just tried running the tests with MT_NO_ISOLATE=1 but there was no difference. I'm using assertion style, and I notice that in my case this only happens where OpenStruct is being used to generate sample input and where XML/JSON responses are generated.

Sample console output from a failing test:

--- expected
+++ actual
@@ -1 +1 @@
-"<chart><series name=\"Lineups\"><name>56</name><name>614</name><name>0</name></series></chart>"
+"<chart><series name=\"Lineups\"><name>#&lt;struct Minitest::Expectation target=nil, ctx=#&lt;OpenStruct name=\"match day #abc\", value=56&gt;&gt;</name><name>#&lt;struct Minitest::Expectation target=nil, ctx=#&lt;OpenStruct name=\"match day #def\", value=614&gt;&gt;</name><name>#&lt;struct Minitest::Expectation target=nil, ctx=#&lt;OpenStruct name=\"match day #012\", value=0&gt;&gt;</name></series></chart>"

Note that the expected values have been replaced with Minitest::Expectation target=nil. The test for this case is building some sample input:

def build_sample_input
  [
    OpenStruct.new(
      :name   => "match day #abc",
      :value  => 56
    ),
    ...
  ]
end

and the code that's being tested uses the input with Nokogiri and builds some XML output.

builder = Nokogiri::XML::Builder.new do |xml|
  process_source(xml)
end

def process_source(xml)
  xml.chart do
    xml.show_graph_ false
    xml.type_ 'line'
    xml.orientation_ 'vertical'
    xml.description_ 'Unique Lineup Selections'
    xml.categories do
     input.each do |match_day|
        xml.name_ match_day.name
     end
    end
    xml.series('name' => 'Lineups') do
      input.each do |match_day|
        xml.name_ match_day.value
      end
    end
  end
end

@ammar
Copy link
Author

ammar commented Apr 17, 2015

Running the test by itself with MT_NO_ISOLATE=1 produced the same error.

My case is similar to @daxadax. The argument in question is a subclass of OpenStruct. However there is no XML or JSON generation, or parsing, involved.

@ammar
Copy link
Author

ammar commented Apr 17, 2015

Correction: the expected argument in my case is not a subclass of OpenStruct, it is a value from a subclass of OpenStruct. It's the label value of what ctx points to:

#<struct Minitest::Expectation target=nil, ctx=#<Syntax::PDF::Stylesheet label={:size=>10, :italic=>true, :color=>"444444"}, value={:size=>10, :bold=>true}>>

Syntax::PDF::Stylesheet is the subclass of OpenStruct.

@daxadax
Copy link

daxadax commented Apr 17, 2015

@ammar, try running with with MT_NO_EXPECTATIONS=1

My colleague (@splattael) helped me to work out the minimum code necessary to generate failure and in the process we discovered that this seems to be related to Minitest::Expectations making an alias for value, expect and _.

After setting the MT_NO_EXPECTATIONS environment variable, the code runs with no errors as you can see here.

@zenspider
Copy link
Collaborator

Ok. Now that we've verified that part, can you chase that down and find a repro? Replacing _ with a method that raises should be expedient.

I think it needs to move to its own module and/or into Spec.

On Apr 17, 2015, at 06:58, daxadax notifications@github.com wrote:

@ammar, try running with with MT_NO_EXPECTATIONS=1

My colleague (@splattael) helped me to work out the minimum code necessary to generate failure and in the process we discovered that this (seems to) be related to Minitest::Expectations making an alias for value, expect and _.

After setting the MT_NO_EXPECTATIONS environment variable, the code runs with no errors as you can see here.


Reply to this email directly or view it on GitHub.

@ammar
Copy link
Author

ammar commented Apr 17, 2015

@daxadax, running with MT_NO_EXPECTATIONS=1 worked.

@zenspider, thanks for the hint. Here's a repro I found after replacing _ with a method that raises. Testing the aliases is redundant in this case, I guess.

require 'test_helper'

class ValueMonadTest < Minitest::Test
  def setup
    @struct = OpenStruct.new(_: '_', value: 'VALUE', expect: 'EXPECT')
  end

  def test_value_monad_method
    assert_equal '_', @struct._
  end

  def test_value_monad_value_alias
    assert_equal 'VALUE', @struct.value
  end

  def test_value_monad_expect_alias
    assert_equal 'EXPECT', @struct.expect
  end
end

@zenspider
Copy link
Collaborator

Fuckin' openstruct... it's always openstruct... ugh.

@zenspider
Copy link
Collaborator

Hrm... ok. How about a quick patch:

class Object
  private :_
end

I'm still leaning towards pushing _ down somewhere. I'd just like to find a quicky for people stuck with this for now.

@ammar
Copy link
Author

ammar commented Apr 17, 2015

The patch worked for _ but not the aliases. I added them to the patch and they passed too.

class Object
  private :_, :value, :expect
end

I also ran the whole suite with that patch applied and it passed completely.

@zenspider
Copy link
Collaborator

OK. Now that we know what is going on. We need a fix...

@zenspider
Copy link
Collaborator

How about something like: https://gist.github.com/473ce2f403d28eb5f46b ?

@ammar
Copy link
Author

ammar commented Apr 17, 2015

That patch did the trick. The repro and the suites from a few different apps all pass as before. Cheers!

@blowmage
Copy link

👏

@zenspider
Copy link
Collaborator

Sorry. I committed this a few days ago. I'll release sometime soon.

esamattis added a commit to puavo-org/puavo-web that referenced this issue May 5, 2015
Breaks build on CI for some reason

See minitest/minitest#521
@minitest minitest locked and limited conversation to collaborators May 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

4 participants