Add Roda instrumentation

[hannahramadan]

Adds instrumentation for Roda versions 3.19.0+. Roda will be automatically detected and instrumented.

closes #1783

Original change not required (I didn't catch the ||), but the refactor looks great so we'll keep it.

[fallwith]

Following up on offline discussions on this one...

• Minitest doesn't support (and one might even infer it discourages) the stubbing of any instance
• The any_instance here is provided by the Mocha testing gem
• Personally I think introducing Mocha to a Minitest based project detracts from the Minitest experience as much as it adds to it. Perhaps it's like adding a sidecar to a motorcycle. It's clear that the sidecar offers a distinct advantage over a motorcycle without a sidecar. But there are also fundamental changes to the motorcycling experience that could be considered as drawbacks.
• Testing any instance is well known anti-pattern given the risk of having stubbing behavior bleed over to other instances in play in a testing context where the stubbing behavior would be problematic.
  • The risk is mitigated here in two ways:
    • We are assured of only having one Rack request in play during the test.
    • We do not (currently) make use of parallelized, concurrent tests.
• Typically the best practice is to have a handle on the specific instance under test. The need to reach for the stubbing of any instance could itself indicate complexity of code. This complexity might come from not following TDD or could indicate a focus / prioritization on performance over testability. For whatever reason, tests that require stubbing any instance might indicate difficulties in maintainability.
• We use this any_instance method quite a lot in this project. It is completely acceptable to use it again here from a PR approval standpoint. The maintainers may at one point decide to dissuade future usage or refactor out existing usage of any_instance, but for now there's a green light.
  • We bring in a RuboCop gem specific to Minitest, but unfortunately given that any_instance comes from Mocha, its usage is not flagged by any cops.
• Looking at the context for this exact line of code, the benefit of using any_instance is pretty clear. We invoke the Rack::Test get method, which via Forwardable goes through the Rack::Test flow of calling the user defined app method to get an app class. An instance of that class is instantiated and the request params object is prepped. For us to be able to get at the exact single instance of the class (as opposed to any instance) that will field the web request, we would need to change the overall flow of this test and have it be different from all the others. Perhaps we could stub new on the Rack app and use that stubbing for a foothold while retaining the Rack test integration style of using get. Alternatively, we could have this test not leverage Rack::Test and we could instead directly invoke the rack_request_params method being tested. (We'd use a unit test instead of an integration test). The benefits of having this test look and act like the others is a pretty clear advantage of any_instance.

[hannahramadan]

Thanks for the detailed thoughts—loving the motorcycle analogy. I agree it's weird to mix Mocha and Minitest. I've updated this and to go with the unit-style test— just calling rack_request_params directly, which is going to fail since no Rack request will be in existence.

Updating our codebase to get rid of any_instance would be a fun project. I think it would help with learning.

[github-actions]

SimpleCov Report

	Coverage	Threshold
Line	94.29%	94%
Branch	85.78%	85%

[kaylareopelle]

Great work, Hannah!