-
Notifications
You must be signed in to change notification settings - Fork 160
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
Fix Test
to correctly handle test outputs which do not end with a newline when using rewriteToFile
#3695
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not actually tested this PR, but the description seems useful, and the code looks plausible to me. Thanks!
@@ -633,6 +633,12 @@ InstallGlobalFunction("Test", function(arg) | |||
od; | |||
if PositionSublist(pf.inp[i], "STOP_TEST") <> 1 then | |||
Append(new[pf.pos[i]], pf.cmp[i]); | |||
if pf.cmp[i] <> "" and Last(pf.cmp[i]) <> '\n' then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or you could do this, and then we could backport this to stable-4.11 (however, I am perfectly fine with merging this as-is):
if pf.cmp[i] <> "" and Last(pf.cmp[i]) <> '\n' then | |
if pf.cmp[i] <> "" and not EndsWith(pf.cmp[i], "\n") then |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question (just to check my understanding of the code, which is limited): When is pf.cmp[i]
an empty string? If a command is expected to not output anything, maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeps, tests with no output will have pf.cmp[i]
empty. This won't cause a problem as we will just start outputting the next input on the line after the previous input. It's just when the line is non-empty but not \n
terminated.
@ChrisJefferson feel free to merge this at any time, though it'd be nice if you could set at least the appropriate "release notes" label (and if you deem this should be added to release notes, also make sure the PR title can be copied verbatim to release notes for users). Thanks! |
Added labels, tweaked titles. I don't think this really needs backporting, as it is a very long-standing issue, and (hopefully) 4.11 should only get required fixes now really? |
Test
to correctly handle test outputs which do not end with a newline when using rewriteToFile
Given a statement in a .tst file which does not end in a newline, rewriteToFile gets very confused, and in particular can "lose" lines of output if run over and over again. This just prints a warning, and inserts a newline.
As an example of the problem, consider this test file:
Running test with rewriteToFile gives:
Running again on this gives the following, where the
1
has been lost