jejacks0n / teaspoon

Teaspoon: Javascript test runner for Rails. Use Selenium, BrowserStack, or PhantomJS.
1.43k stars 243 forks source link

Code coverage not working with Puma 6.2 #600

Closed ukdave closed 10 months ago

ukdave commented 10 months ago

I just upgraded Puma from from 6.1 to 6.2 and suddenly all my teaspoon tests failed. After quite a bit of digging I found out it was to do with the code coverage - specifically how the assets are instrumented.

I added this monkey patch to my teaspoon_env.rb file which seems to fix it:

module Teaspoon
  class Instrumentation
    def instrumented_response
      status, headers, asset = @response
      headers, asset = [headers.clone, asset.clone]

      result = add_instrumentation(asset)
      headers["Content-Length"] = result.bytesize.to_s
      [status, headers, [result]]
    end
  end
end

I'm thinking the issue is due to PR #3072 in Puma and the original Teaspoon::Instrumentation#instrumented_response method returning an Asset object which has file-like methods (e.g. #filename and #bytesize). I'm wondering if Puma 6.2 is trying to read the original asset file, but is getting thrown off because we've updated the content-length. Either way changing the instrumented_response method to return the modified file contents as a string seems to fix it.

mathieujobin commented 10 months ago

nice ! I would have never thought.... this is really helpful.

would you mind opening a PR with the fix ? is it backward compatible with puma 6.1 and below or we need a conditional ?

mathieujobin commented 10 months ago

cc: @phenchaw

ukdave commented 10 months ago

I've tested and confirmed that the fix works with Puma 6.3.1, 6.2.2, 6.1.1, 6.0.1, 5.6.7, and 5.0.4. I've opened PR #601

mathieujobin commented 10 months ago

truly awesome, thanks

mathieujobin commented 10 months ago

released in v1.4.0 https://rubygems.org/gems/teaspoon/versions/1.4.0