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

Include ActiveSupport::Testing::TaggedLogging for Rails >= 7 #2580

Closed
wants to merge 6 commits into from
Closed

Include ActiveSupport::Testing::TaggedLogging for Rails >= 7 #2580

wants to merge 6 commits into from

Conversation

shanecav84
Copy link
Contributor

Fixes #2545. Not sure if anyone got to this yet. There was also another problem mentioned in that issue about a missing 'name' method, which this PR does not address.

Copy link
Member

@pirj pirj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.
Do you think it's possible to add a snippet that would fail without this fix? An example snippet.

@shanecav84
Copy link
Contributor Author

I've added the snippet. Not quite sure if I've done it correctly. The snippet will fail without the fix.

@pirj
Copy link
Member

pirj commented Mar 11, 2022

Perfect, thank you!

@pirj
Copy link
Member

pirj commented Mar 11, 2022

I've re-triggered the build as Rails seems to have finally released 5.2.x with a fix for Ruby 2.2 support.

But they also released 6.1.5 and broke something for us.

Do you think this is related?

[ActiveJob] Failed enqueuing TestJob to Test(default): TestError (TestError)
F

Failures:

  1) Foo error raised in perform_enqueued_jobs with block raises the explicitly thrown error
     Failure/Error:
       expect { perform_enqueued_jobs { TestJob.perform_later } }
         .to raise_error(expected_error)

       expected TestError, got #<Minitest::UnexpectedError: Unexpected exception> with backtrace:
         # include_activesupport_testing_tagged_logger.rb:71:in `block (5 levels) in <main>'
         # include_activesupport_testing_tagged_logger.rb:71:in `block (4 levels) in <main>'
         # include_activesupport_testing_tagged_logger.rb:71:in `block (3 levels) in <main>'
     # include_activesupport_testing_tagged_logger.rb:71:in `block (3 levels) in <main>'

@pirj pirj requested a review from JonRowe March 11, 2022 20:02
@shanecav84
Copy link
Contributor Author

Not sure what's up with that failing jRuby test.

@pirj
Copy link
Member

pirj commented Mar 14, 2022

Quite an interesting case.
@headius Would you like to have a look?

@JonRowe
Copy link
Member

JonRowe commented Mar 14, 2022

I tried to install JRuby this morning to take a look at this failure, but it seems broken on M1 macs via asdf at the moment, do we need this to be a snippet? Its currently a regression test as well, what actually does the tagged logger do? Could we write an integration test instead?

@headius
Copy link

headius commented Mar 14, 2022

@JonRowe Latest JRuby 9.3 should be working well on M1. Perhaps asdf is not updated to the latest release? (9.3.3).

The failure error is very peculiar. I don't know why TestError would not have the backtrace method under any circumstance. I do notice it is running 9.3.1.0, which is a couple releases old now.

I'll try to reproduce locally.

@headius
Copy link

headius commented Mar 14, 2022

I do see now that it appears to be trying to call backtrace against the TestError class, not against an instance. Unsure why that is happening so far.

@headius
Copy link

headius commented Mar 14, 2022

I have not been able to reproduce so far, but I got some other weird errors that seem to be an issue in my environment.

I would suggest trying to upgrade to 9.3 in this PR since it should be upgraded anyway and might fix the issue.

@shanecav84
Copy link
Contributor Author

Upgrading JRuby did not fix the issue. See also 5d9d3b0 where I instantiated the expected error to get around the JRuby error but that caused other errors.

@headius
Copy link

headius commented Mar 16, 2022

I have cleaned up my local env issue and can now reproduce the same error!

@headius
Copy link

headius commented Mar 17, 2022

I will try to look into this for a fix for JRuby 9.3.4 but no promises... we are hoping to release next week. If anyone can narrow this down to some specific test that fails, it would be helpful. It almost looks like it is happening after the suite completes, but I am unfamiliar with your suites' output.

@shanecav84
Copy link
Contributor Author

@headius

  1. Checkout the branch for this PR
  2. Run bundle
  3. cd snippets; ruby include_activesupport_testing_tagged_logger.rb will run the test.

@pirj
Copy link
Member

pirj commented Jul 26, 2022

Superseded by #2587
Thank you for the hard work!

@pirj pirj closed this Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

undefined local variable or method `tagged_logger' during perform_enqueued_jobs
4 participants