-
Notifications
You must be signed in to change notification settings - Fork 54
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
RSpec hanging when matching on http response #160
Comments
Exactly the same problem and solution. the use of
|
We are experiencing this as well. It appears to be a problem with trying to inspect the SuperDiff.inspect_object(response, as_lines: false) If it helps any, I found that rspec had the same issue with this object, and they fixed it with this rails PR. There's another bit of a hacky fix by changing the RecursionGuard. In this method, if I keep a counter, and guard against that counter hitting too high of a value, we can break out of the infinite loop. Proof of concept code: def self.guarding_recursion_of(*objects, &block)
Thread.current["recursion_counter"] ||= 0
Thread.current["recursion_counter"] = Thread.current["recursion_counter"] + 1
already_seen_objects, first_seen_objects = objects.partition do |object|
!SuperDiff.primitive?(object) && (Thread.current["recursion_counter"] > 1000 || already_seen?(object))
end
# rest of the method is unchanged |
Ah, thanks @travatomic, that's helpful. I'll take a look at the Rails fix — maybe |
For anyone else having this problem: I have two solutions. Both solutions work in the user code and don't need a change to the gem. My company is probably going with option 1. Solution 1:Monkey patch the TestResponse. Add this to the bottom of class ActionDispatch::TestResponse
def attributes_for_super_diff
{
status: status,
# you could also add headers or body here if those are meaningful to you.
}
end
end Solution 2:Create a new inspection tree builder. Make the following new file at spec/support/super_diff/action_dispatch_test_response_inspection_tree_builder.rb class ActionDispatchTestResponseInspectionTreeBuilder < SuperDiff::ObjectInspection::InspectionTreeBuilders::Base
def self.applies_to?(value)
value.is_a?(ActionDispatch::TestResponse)
end
def call
SuperDiff::ObjectInspection::InspectionTree.new do
as_lines_when_rendering_to_lines(collection_bookend: :open) do
add_text { |object| "#<#{object.class} " }
when_rendering_to_lines { add_text "{" }
end
nested do |object|
response_hash = {
status: object.status,
}
insert_hash_inspection_of(response_hash)
end
as_lines_when_rendering_to_lines(collection_bookend: :close) do
when_rendering_to_lines { add_text "}" }
add_text ">"
end
end
end
end Then change your SuperDiff configuration to use the new inspection tree builder: SuperDiff.configure do |config|
config.add_extra_inspection_tree_builder_classes(
ActionDispatchTestResponseInspectionTreeBuilder
)
end OutputWith either option, the output looks something like this:
@mcmire do you think either one of these is suitable to add to the gem? |
I ran into this today using gem version 0.10.0. Will add one more workaround: expect(response.successful?).to be true It's a little less fancy, but seems to not have issues when the test fails. By the way, I am not getting "hanging" behavior. For the code below: expect(response).to be_successful? I just get an error like:
|
Also please note that #159 (comment) seems to reference the same issue, in case that is helpful. |
This just bit me and took some time to debug. This issue has been around a while. @mcmire do you have any thoughts about applying a fix to the gem? |
Bumping this as well, my company is facing this issue as well. A fix to the gem would be great. |
I agree this has been around a while — sorry for the delay on this. I'll try to look at this soon! |
@panozzaj , sorry for the delay. The Re: hanging on the HTTP response – as mentioned in #204, it would be good to find a way to avoid prematurely calculating the Since this gem is designed to support Rails and |
The issue seems to be because of circular references in the object in question, so it seems like we could just wrap the E.g.: # in SuperDiff::OperationTreeBuilders::Base
def compare(expected, actual)
RecursionGuard.guarding_recursion_of(expected, actual) do |already_seen|
next if already_seen
OperationTreeBuilders::Main.call(
expected: expected,
actual: actual,
all_or_nothing: false
)
end
end |
Actually the problem here seems to be in
The issue seems to disappear if the RecursionGuart is wrapped around this line from RecursionGuard.substituting_recursion_of(object) do
t4.add_inspection_of object.instance_variable_get(name)
end |
Thanks for the investigation, @b-loyola ! I'm admittedly not 100% on inspection tree construction and rendering yet, but I believe the inspection tree builder is supposed to be lazy. The recursion guards should be more useful during rendering than during inspection tree construction. I'll take a look at |
On further investigation with a couple production apps, I've come to the conclusion that this is not infinite recursion: it's deep-inspecting
The Rails code itself aliases I added an inspection tree builder for Adding a custom inspection of |
We've seen an issue where in a rails request spec we have something like this:
In cases where the spec would fail, it seems to hang forever (I'm not sure if it would eventually complete if I left it, but it hits our CI pipeline timeout limit of 15 minutes).
I've managed to trace it to a method within super_diff when its trying to report the failure, but I've not found the root cause.
We're using version super_diff 0.9.0, with rspec 3.11.0 (and rails 7.0.4). (I can provide our Gemfile.lock if that would be useful).
If anyone else comes across this with the same problem, we've worked around this by using
have_http_status(:created)
instead.The text was updated successfully, but these errors were encountered: