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.

Sign up for GitHub

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

Jump to bottom

Fix Test to correctly handle test outputs which do not end with a newline when using rewriteToFile #3695

Merged
merged 1 commit into from
Oct 15, 2019
Merged

Fix Test to correctly handle test outputs which do not end with a newline when using rewriteToFile #3695

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions lib/test.gi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

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):

Suggested change
if pf.cmp[i] <> "" and Last(pf.cmp[i]) <> '\n' then
if pf.cmp[i] <> "" and not EndsWith(pf.cmp[i], "\n") then

Copy link
Member

@fingolfin fingolfin Oct 9, 2019 •
edited
Loading

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?

Copy link
Contributor Author

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.

Info(InfoWarning, 1, "An output in the .tst file does not end with a newline. GAP does not support this.");
Info(InfoWarning, 1, "A newline will be inserted to make the file valid, but the test will fail.");
Info(InfoWarning, 1, "The location of this problem is marked with '# Newline inserted here'.");
Append(new[pf.pos[i]], "# Newline inserted here by 'rewriteToFile'\n");
fi;
fi;
od;
new := Concatenation(Compacted(new));
Expand Down