-
-
Notifications
You must be signed in to change notification settings - Fork 547
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
Transpose: add test-case #1046
Comments
Interesting. I am not seeing how the new test is different from the two cases you mentioned. Is it the fact that you are proposing three lines instead of two? Why doesn't I have not attempted this exercise, so I have no familiarity with it other than the documentation and test data. |
The difference is, that this test is the only one that checks if the right-side-space-trimming works on more than just the last line in "expected". "Many lines" does not cover this, since the last line of input is too long (you can see that only the last line in "expected" needs to get right-trimmed) |
I just came up with an even better test-case for this: {
"description": "mixed line length",
"property": "transpose",
"input": [
"The longest line.",
"A long line.",
"A longer line.",
"A line."
],
"expected": [
"TAAA",
"h ",
"elll",
" ooi",
"lnnn",
"ogge",
"n e.",
"glr",
"ei ",
"snl",
"tei",
" .n",
"l e",
"i .",
"n",
"e",
"."
]
} Maybe @ErikSchierboom or @rbasso want to review this? |
I think this is a very nice addition. It may even be a replacement for the existing "many lines" case, as this case handles the same problem but in a better, more interesting way. |
The current version of the transpose-test cases (1.0.0) seem to miss out on a case:
There are two test-cases which cover the correct right-stripping of white-space
("first line longer than second line" and "many lines"), yet they do not cover the case, where you have to right-trim whitespace with multiple lines at the end (Idk if i describe it correctly).
I noticed it, when I checked some implementations in the Python track which failed my following suggested test:
The text was updated successfully, but these errors were encountered: