YAML.jl icon indicating copy to clipboard operation
YAML.jl copied to clipboard

A test `windows_newlines` seems wrong

Open Paalon opened this issue 1 year ago • 10 comments

From #188. I found prefix(::BufferedInput, ::Integer) has a bug and if I fixed it, the test windows_newlines fails. Also, better implementation in #188's TODO fails at the same test. I think the test windows_newlines is something wrong.

Paalon avatar Jun 17 '24 07:06 Paalon

The content:

julia> str = read("test/windows_newlines.data", String)
"hello:\r\0\r"

Paalon avatar Jun 17 '24 09:06 Paalon

Loading result of windows_newlines from http://ben-kiki.org/ypaste/cgi-bin/ypaste.pl

スクリーンショット 2024-06-17 18 54 00

Paalon avatar Jun 17 '24 09:06 Paalon

So the test should be fixed.

Paalon avatar Jun 17 '24 09:06 Paalon

I don't know why only 2 tests are failed. スクリーンショット 2024-06-17 19 07 22

Paalon avatar Jun 17 '24 10:06 Paalon

It's strange to use hello:\r\0\r to test for Windows. It should be hello:\r\n\r.

Paalon avatar Jun 17 '24 10:06 Paalon

I asked this in YAML community, and the file hello:\r\0\r is malformed. We should fail this even for load_all_file and load_file and load but only load_all fails. Hmm, why?

Paalon avatar Jun 17 '24 10:06 Paalon

This test file was added when #17 was fixed. I think it was just supposed to be hello:\r\n but ended up differently.

GunnarFarneback avatar Jun 17 '24 20:06 GunnarFarneback

#211 fixes this.

Paalon avatar Jun 18 '24 04:06 Paalon

We should fail this even for load_all_file and load_file and load but only load_all fails. Hmm, why?

It's probably incorrect, but \0 is basically parsed like \n..., so this document is like hello:\r\n...\r. load_all fails because the second document has no content, which is known to be handled badly. load_file and load work because they stop after the first document. load_all_file works thanks to the bug pointed out in #120. YAMLDocIterator loads the first document on construction, and the second document when the first is retrieved. Due to the input file being prematurely closed, it never tries to load the second document as the closed input signals eof.

GunnarFarneback avatar Jun 18 '24 20:06 GunnarFarneback

is this closed by #218 ?

kescobo avatar Jun 25 '24 13:06 kescobo