-
Notifications
You must be signed in to change notification settings - Fork 191
Supress expected warning for test suite #604
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
Conversation
@@ -15,13 +15,12 @@ class SolidQueue::FailedExecutionTest < ActiveSupport::TestCase | |||
end | |||
|
|||
test "run job that fails with a SystemStackError (stack level too deep)" do | |||
silence_on_thread_error_for(SystemStackError) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we were already trying to do this, but with no success.
From my research, only exceptions who do not inherit StandardError
are being logged.
ExpectedTestError
inherits RuntimeError
which inherits StandardError
.
The problem with SystemStackError
is that it inherits Exception
- not StandardError
- which leaks a log. Not 100% sure why though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh yes! I had forgotten we were trying to do this here, because it didn't work 😅
The intended goal for that test was to have a gigantic backtrace generated by a solid_queue/app/models/solid_queue/failed_execution.rb Lines 53 to 55 in f433a99
|
Hey @rosa No, it doesn't fail. Nothing is really testing that backtrace truncation you mentioned, which made me notice something quite interesting, unrelated to my change: The truncation depends on the solid_queue/app/models/solid_queue/failed_execution.rb Lines 60 to 65 in 74b12c8
But solid_queue/lib/generators/solid_queue/install/templates/db/queue_schema.rb Lines 22 to 27 in 74b12c8
solid_queue/test/dummy/db/queue_schema.rb Lines 34 to 39 in 74b12c8
Truncation is not actually happening on |
The test failed without the truncation, but it's RDBMS-dependent. It's explained in 262c0c8 |
Ohh that explains it, my bad, I only tested with SQLite and Postgres 😅. When I didn't find |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you so much! 🙏 🥳
I believe we should supress the
ERROR -- stack level too deep: nil
that's currently happening in the test suite:The failure is intentional (we're exercising a
SystemStackError
on purpose), but the noisy log isn’t documented and has already confused contributors into thinking it's a real test failure (see #527).