Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A test windows_newlines seems wrong #207

Open
Paalon opened this issue Jun 17, 2024 · 10 comments
Open

A test windows_newlines seems wrong #207

Paalon opened this issue Jun 17, 2024 · 10 comments

Comments

@Paalon
Copy link
Contributor

Paalon commented Jun 17, 2024

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
Copy link
Contributor Author

Paalon commented Jun 17, 2024

The content:

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

@Paalon
Copy link
Contributor Author

Paalon commented Jun 17, 2024

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

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

@Paalon
Copy link
Contributor Author

Paalon commented Jun 17, 2024

So the test should be fixed.

@Paalon
Copy link
Contributor Author

Paalon commented Jun 17, 2024

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

@Paalon
Copy link
Contributor Author

Paalon commented Jun 17, 2024

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

@Paalon
Copy link
Contributor Author

Paalon commented Jun 17, 2024

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?

@GunnarFarneback
Copy link
Contributor

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

@Paalon
Copy link
Contributor Author

Paalon commented Jun 18, 2024

#211 fixes this.

@GunnarFarneback
Copy link
Contributor

GunnarFarneback commented Jun 18, 2024

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.

@kescobo
Copy link
Collaborator

kescobo commented Jun 25, 2024

is this closed by #218 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants