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

undefined local variable or method `tagged_logger' during perform_enqueued_jobs #2545

Closed
jkwuc89 opened this issue Dec 17, 2021 · 22 comments
Closed
Labels

Comments

@jkwuc89
Copy link

jkwuc89 commented Dec 17, 2021

What Ruby, Rails and RSpec versions are you using?

Ruby version: 3.0.3p157 (2021-11-24 revision 3fb7d2cadc) [x86_64-darwin21]
Rails version: 7.0.0
RSpec 3.10

  • rspec-core 3.10.1
  • rspec-expectations 3.10.1
  • rspec-mocks 3.10.2
  • rspec-rails 5.0.2
  • rspec-support 3.10.3

Observed behaviour

Inside an rspec support module, my app is calling perform_enqueued_jobs. The spec in question that is using this support module is expecting this call to raise an exception.

When I attempt to run this spec, the following is raised:

got #<NameError: undefined local variable or method `tagged_logger'

When I debug into perform_enqueued_jobs, I am taken into _assert_nothing_raised_or_warn inside activesupport-7.0.0/lib/active_support/testing/assertions.rb, When debugging the block below, it shows e as having the expected exception.

        rescue Minitest::UnexpectedError => e
          if tagged_logger && tagged_logger.warn?
            warning = <<~MSG
              #{self.class} - #{name}: #{e.error.class} raised.
              If you expected this exception, use `assert_raises` as near to the code that raises as possible.
              Other block based assertions (e.g. `#{assertion}`) can be used, as long as `assert_raises` is inside their block.
            MSG
            tagged_logger.warn warning
          end

          raise
        end

Debugging further reveals that the if tagged_logger check above is causing the undefined error.

I reported this on Rails forum here and a member of the core team suggested the following:

"This seems to be a bug in RSpec that doesn’t implement the tagged_logger. Please report the issue in the RSpec issue tracker."

Expected behaviour

The spec should finish and allow to raise_error to check for the expected exception. This spec is working as expected in Rails 6.

@pirj
Copy link
Member

pirj commented Dec 17, 2021

Thanks for reporting.
tagger_logger seems to have been introduced in this commit. Wondering if we should work this around by setting it to nil in our adapters code, or this should be fixed in Rails. Wondering where this tagged_logger is defined/initialized in Rails code for testing.
Would you like to dig deeper in this, @jkwuc89 ?

@pirj pirj added the rails7 label Dec 17, 2021
@jkwuc89
Copy link
Author

jkwuc89 commented Dec 17, 2021

I'll take a deeper dive into this and report what I find.

@JonRowe
Copy link
Member

JonRowe commented Dec 18, 2021

It feels like they are checking for a tagged logger by assuming it exists, they could easily fix this by checking its defined

@jkwuc89
Copy link
Author

jkwuc89 commented Dec 20, 2021

Inside the Rails PR that added the block I included in the description above, one finds this comment which suggests checking if tagged_logger is defined. 🤔

rails/rails#42459 (comment)

@rafaelfranca
Copy link

I think rspec-rails should be responsible for making sure the features of ActiveSupport::TestCase it uses works with RSpec, nor Rails that should check if its own API is existent.

@pirj
Copy link
Member

pirj commented Dec 20, 2021

Thanks for the hints @rafaelfranca, @ghiculescu.

@jkwuc89 Does it fix the issue if you add the following to rspec-rails' lib/rspec/rails/example/rails_example_group.rb?

        extend ActiveSupport::Concern
+       include ActiveSupport::Testing::TaggedLogging
        include RSpec::Rails::SetupAndTeardownAdapter

@westonganger
Copy link

westonganger commented Dec 20, 2021

Here is a quick monkey patch using @pirj suggested solution

# spec/rspec_helper.rb

if Rails::VERSION::MAJOR >= 7
  require 'rspec/rails/version'
  
  if Gem::Version.create(RSpec::Rails::Version::STRING) < Gem::Version.create("5.1.0")
    RSpec::Rails::RailsExampleGroup.module_eval do
      include ActiveSupport::Testing::TaggedLogging
    end
  end
end

@jkwuc89
Copy link
Author

jkwuc89 commented Dec 21, 2021

Adding the following to spec/rails_helper.rb inside my Rails project allows tagged_logger to get defined which fixes the original undefined error in the description above.

if Rails::VERSION::MAJOR >= 7
  include ActiveSupport::Testing::TaggedLogging
end

Unfortunately, it causes another issue when I run the spec in question. When _assert_nothing_raised_or_warn is called inside activesupport-7.0.0/lib/active_support/testing/assertions.rb as part of handling an exception during perform_enqueued_jobs, it causes the spec to fail with the following because _assert_nothing_raised_or_warn uses name inside its definition.

expected Workflow::SellerAuthorization::MissingSellerAuthorizationError, got #<RSpec::Core::ExampleGroup::WrongScopeError: `name` is not available from within an example (e.g. an...ore`, `let`, etc). It is only available on an example group (e.g. a `describe` or `context` block).> with backtrace:
  # ./spec/support/events/async_event_processor.rb:6:in `publish_event_inline'
  # ./spec/models/workflow/seller_authorization_spec.rb:668:in `block (3 levels) in <top (required)>'
  # ./spec/models/workflow/seller_authorization_spec.rb:699:in `block (6 levels) in <top (required)>'
  # ./spec/models/workflow/seller_authorization_spec.rb:699:in `block (5 levels) in <top (required)>'
  # ./spec/spec_helper.rb:20:in `block (2 levels) in <top (required)>'

@xxx
Copy link

xxx commented Feb 4, 2022

I'm using the following crappy patch, as including the module into RSpec::Rails::RailsExampleGroup did not do anything.

Also defined a dummy name method (sigh), to resolve the subsequent issue that @jkwuc89 raises just above (which this also runs into)

if Rails::VERSION::MAJOR >= 7
  require 'rspec/rails/version'

  RSpec::Core::ExampleGroup.module_eval do
    include ActiveSupport::Testing::TaggedLogging

    def name
      'foobar'
    end
  end
end

@JonRowe
Copy link
Member

JonRowe commented Feb 4, 2022

PRs would be accepted to solve this, RailsExampleGroup should conditionally (as in, if on Rails 7) include this module, and ideally we'd want a snippet testing it.

@JonRowe
Copy link
Member

JonRowe commented Oct 20, 2022

This was fixed in 6.0.0 and further improved in 6.0.1

@JonRowe JonRowe closed this as completed Oct 20, 2022
@ricardopacheco
Copy link

@JonRowe the problem still persists, version 6.0.1

@JonRowe
Copy link
Member

JonRowe commented Dec 13, 2022

Please provide a reproduction snippet.

@joemsak
Copy link

joemsak commented Mar 8, 2023

I am getting this error in 6.0.1 with this test:

it "test description" do
  expect {
    perform_enqueued_jobs do
      # failing LOC here, due to raised exception (uncaught, unexpected)
    end
  }.to change { ActionMailer::Base.deliveries.size }.by(2)
end

I get just enough of the actual backtrace in the stdout to then pry in and figure out the exception / failure myself but yeah, ultimately it's slowing me down

@riffraff
Copy link

I also see the same error with 6.0.1, my code looks similar

      specify do
        expect { perform_enqueued_jobs { dispatch } }
          .to not_change(ActionMailer::Base.deliveries, :size)
          .and not_raise_error

Ruby 3.2.1, rails 7.0.4

@pirj
Copy link
Member

pirj commented May 21, 2023

Does the monkey patch mentioned above fix it for you @riffraff @joemsak ?

It would be very helpful if you could provide a runnable snippet to reproduce the issue. doc, examples.

Just for the reference, related PRs: #2587, #2625.

@bigxiang
Copy link

@pirj I just saw this error in 6.0.1, and the monkey patch above #2545 (comment) can fix the issue.

However, I find I missed type: :job in my spec. If I add it, everything works fine. I think it's because the error is raised by RSpec::Core::ExampleGroup if I don't add type: :job, so the monkey patch works. When running a spec with type: :job, RSpec::Rails::RailsExampleGroup is included, so the fix works. Does it make sense?

@pirj
Copy link
Member

pirj commented Jun 16, 2023

If it’s not a Rails example group (no ‘type:’ set), why it is expected to include the code introduced in #2587 ?
I would refrain from including this to non-Rails example groups.
Am I missing something?

@brianaj
Copy link

brianaj commented Jul 17, 2023

Still see this issue and #2545 (comment) helps resolve it but would like to understand why #2587 doesn't fix things?

@pirj
Copy link
Member

pirj commented Jul 18, 2023

Because this is only for Rails example groups. But the patch you refer to does patch it for all example groups.
Do you add proper metadata to your Raisl-related example groups?

@sherryptk
Copy link

I'm running this code:

describe "#request_token" do
it "returns a successful response" do
post :request_token, params: { username: 'XXX', password: 'XXX' }
expect(response).to have_http_status(:success)
end
end

and am getting this error:

=> There was an error building fixtures
=> #<ArgumentError: wrong number of arguments (given 0, expected 1+)>

/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/bundler/gems/fixture_builder-7a042cc63699/lib/fixture_builder/namer.rb:16:in name' /.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/bundler/gems/fixture_builder-7a042cc63699/lib/fixture_builder/delegations.rb:16:in name'
/.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7.2/lib/active_support/testing/assertions.rb:254:in rescue in _assert_nothing_raised_or_warn' /.rbenv/versions/3.2.2/lib/ruby/gems/3.2.0/gems/activesupport-7.0.7.2/lib/active_support/testing/assertions.rb:249:in _assert_nothing_raised_or_warn'

I got past the first "tagged logger" error by adding include ActiveSupport::Testing::TaggedLogging, however I'm seeing this afterward and haven't seen anything addressing it. Adding def name did not work for me.

@pirj
Copy link
Member

pirj commented Oct 23, 2023

I see some ‘fixture_builder’ gem in the log. And i see you’ve filed a bug there, too rdy/fixture_builder#70

Can you check if this branch would fix your issue? rdy/fixture_builder#68

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.