Skip to content

Conversation

@sambostock
Copy link
Contributor

While dalli doesn't depend on resolv-replace, we want to ensure it still works if it is required by the host application. (#989)

This regression test ensures we don't break compatibility, and adds a framework allowing us to test compatibility with other gems in the future.

@sambostock sambostock force-pushed the resolv-replace-regression-test branch 3 times, most recently from 9fc690a to 09dea6d Compare March 8, 2024 04:52
While `dalli` doesn't depend on `resolv-replace`, we want to ensure it
still works if it is required by the host application.

This regression test ensures we don't break compatibility, and adds a
framework allowing us to test compatibility with other gems in the
future.
@sambostock sambostock force-pushed the resolv-replace-regression-test branch from 09dea6d to 610a4b9 Compare March 8, 2024 05:10
Comment on lines +3 to +4
# JRuby does not support forking, and it doesn't seem worth the effort to make it work.
return unless Process.respond_to?(:fork)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Active Support has some workarounds for this we could use, but it doesn't seem worth it for a code path that was previously untested, and is unlikely to behave differently on JRuby.

].each do |gem_name|
it "passes smoke test with #{gem_name.inspect} gem required" do
memcached(:binary, rand(21_397..21_896)) do |_, port|
in_isolation(timeout: 10) do
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We want to be able to require the gem in isolation so as not to affect the host process and other tests.

assert_equal(expected, client.get(key), message)
end

def in_isolation(timeout:) # rubocop:disable Metrics
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This method triggers several offences in the Metrics department, but I don't know that splitting it would be an improvement.

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.

1 participant