Newer SnakeYAML defers emit waiting for comments

[headius]

In https://bitbucket.org/asomov/snakeyaml/pull-requests/7 SnakeYAML added features to support parsing and emitting comments. Unfortunately this has had a side effect of deferring some emits, and there does not appear to be a way to turn it off.

An example from https://github.com/jruby/jruby/pull/6680 shows how this is failing:

      def test_float
        @stream.push 1.2
        assert_match(/^--- 1.2/, @io.string)
      end

  2) Failure:
Psych::JSON::TestStream#test_float [/home/travis/build/jruby/jruby/test/mri/psych/json/test_stream.rb:50]:
Expected /^--- 1.2/ to match "".

The issue here is that since the stream is never finished and SnakeYAML is waiting for a comment, the emit of the float scalar never happens. Modifying the code to do @stream.finishafter the push allows this and all similar tests to pass.

So the question is: should the tests or SnakeYAML be modified?

Basically I need to understand whether this deferred emit constitutes a significant-enough break from libyaml to warrant a fix to SnakeYAML, or if the tests are unreasonable in expecting all events to be emitted immediately.

Any input from other Psych maintainers?

[headius]

FIled https://bitbucket.org/asomov/snakeyaml/issues/503/comment-handling-causes-deferred-emit

[headius]

Patch that would fix the tests in question:

diff --git a/test/mri/psych/json/test_stream.rb b/test/mri/psych/json/test_stream.rb
index 90a770c1b7..5abd2a538d 100644
--- a/test/mri/psych/json/test_stream.rb
+++ b/test/mri/psych/json/test_stream.rb
@@ -27,26 +27,31 @@ module Psych
 
       def test_null
         @stream.push(nil)
+        @stream.finish
         assert_match(/^--- null/, @io.string)
       end
 
       def test_string
         @stream.push "foo"
+        @stream.finish
         assert_match(/(["])foo\1/, @io.string)
       end
 
       def test_symbol
         @stream.push :foo
+        @stream.finish
         assert_match(/(["])foo\1/, @io.string)
       end
 
       def test_int
         @stream.push 10
+        @stream.finish
         assert_match(/^--- 10/, @io.string)
       end
 
       def test_float
         @stream.push 1.2
+        @stream.finish
         assert_match(/^--- 1.2/, @io.string)
       end
 
@@ -55,6 +60,7 @@ module Psych
         @stream.push hash
 
         json = @io.string
+        @stream.finish
         assert_match(/}$/, json)
         assert_match(/^--- \{/, json)
         assert_match(/["]one['"]/, json)
@@ -66,6 +72,7 @@ module Psych
         @stream.push list
 
         json = @io.string
+        @stream.finish
         assert_match(/\]$/, json)
         assert_match(/^--- \[/, json)
         assert_match(/["]one["]/, json)
@@ -96,6 +103,7 @@ module Psych
         time = Time.utc(2010, 10, 10)
         @stream.push({'a' => time })
         json = @io.string
+        @stream.finish
         assert_match "{\"a\": \"2010-10-10 00:00:00.000000000 Z\"}\n", json
       end
 
@@ -103,6 +111,7 @@ module Psych
         time = Time.new(2010, 10, 10).to_datetime
         @stream.push({'a' => time })
         json = @io.string
+        @stream.finish
         assert_match "{\"a\": \"#{time.strftime("%Y-%m-%d %H:%M:%S.%9N %:z")}\"}\n", json
       end
     end

[SaltyAimbOtter]

As commented in the filed issue, updating to snakeYAML 1.29-SNAPSHOT should solve this problem.

[asomov]

@headius can you please try the latest snapshot of SnakeYAML ?

[headius]

@asomov I will check it out, thank you!

[headius]

Using a snapshot build of snakeyaml 1.29 does appear to fix the delayed output. I will work with the maintainers to try to get a release we can use.

[headius]

@hsbt @tenderlove If we can get a release of SnakeYAML 1.29, would it be possible to push an updated 3.3 release? The changes in 4.0 are a bit too drastic (safe_load etc) but we would like to pick up the fixed SnakeYAML in a 3.3 version.

[tenderlove]

@headius ya of course!

[tenderlove]

I've pushed a 3-3-stable branch so we can backport changes there.

[headius]

@tenderlove Thank you!

[asomov]

@headius SnakeYAML released

[headius]

I'm not sure if or how I screwed up verifying the snapshot, but the released SnakeYAML 1.29 does not appear to fix my issue. It still appears to defer output until finish. I will follow up with them.

[headius]

In the SnakeYAML issue listed above I identified a test that would show the broken behavior, but it was patched to work around the issue. Hopefully they will be able to address the root cause now.

[hsbt]

I released 4.0.5 and 3.3.3 with SnakeYAML 1.31.

• https://github.com/ruby/psych/releases/tag/v4.0.5
• https://github.com/ruby/psych/releases/tag/v3.3.3

@headius Can we still get this issues?

[headius]

Oh yes, I will check on that!

[asomov]

@headius feel free to provide the info

[headius]

With latest psych master (using latest SnakeYAML) the failing tests are gone!

I had to commit a small patch to mimic @tenderlove's changes in #580 but with that fixed we have 5F7E. About half of those appear to be differences in line/column position reporting in SnakeYAML, and the rest are a mix of errors I will try to look into.

I will call this one fixed.