search

ruby / psych

A libyaml wrapper for Ruby
MIT License
566 stars 206 forks source link

YAML 1.2 compliance possibly breaking VCR #648

Open oliverbarnes opened 1 year ago

oliverbarnes commented 1 year ago

This is an issue branching off from https://github.com/ruby/psych/pull/613#issuecomment-1709239243.

We're experimenting migrating a Rails app we have to latest JRuby (9.4.3.0), an app that has some large VCR fixtures.

Initially we hit the the code point limit issue that https://github.com/ruby/psych/pull/613 lets us bypass, but after increasing the code point limit that the above PR let's us configure, we came across parsing errors like this one:

the leading empty lines contain more spaces (8) than the first non-empty line. while scanning a block scalar at line 32385 column 5
     # ./config/initializers/psych.rb:6:in `parse_stream'

(the initializer monkey patch is a hack while we can't get https://github.com/ruby/psych/pull/647 to work on our end)

In this case, the fixture had several empty newlines in a multiline field, using a block scalar header like field: |+2.

After deleting that field from the yaml fixture altogether as part of troubleshooting, all fixtures get parsed and we're able to run Rspec specs again using VCR. But some of the specs that were previously passing now fail, with Webmock not recognizing the URIs for the requests recorded in the fixtures.

We suspect this might be happening due to YAML 1.2 not liking how the URIs are escaped, and we're still validating this. @headius asked me to create this issue so it can be tracked together with https://github.com/ruby/psych/issues/642, which might also be related to YAML 1.2 compliance (see also https://github.com/jruby/jruby/pull/7600#issuecomment-1712771885)

I'll update this issue as we find out more about it, ideally with a test comparing the same use case with YAML 1.1 and YAML 1.2.

oliverbarnes commented 1 year ago

After some more digging, it seems that the URIs are in fact recognized, but VCR cassetes are "played" more times than the fixture recorded for. But we couldn't figure out why so far.

Unfortunately we won't have more time to look into this for now, as we're pausing our JRuby spike. If we get back to it, we'll try to continue investigating this.

headius commented 4 months ago

Just following up to see if there's any new updates. Anything I can do to help with this?

oliverbarnes commented 4 months ago

thanks for checking in @headius!

We revisited jruby recently, but we're still a ways off from taking the dive. If/once we do, we'll be sure to touch on this again and I'll update the issue then