Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/20272 )
Change subject: IMPALA-12311: Do not add a trailing newline when updating a golden file ...................................................................... Patch Set 1: (1 comment) > Patch Set 1: > > (1 comment) Thanks for the feedback Riza! I will try to see how to address your comment. http://gerrit.cloudera.org:8080/#/c/20272/1/tests/util/test_file_parser.py File tests/util/test_file_parser.py: http://gerrit.cloudera.org:8080/#/c/20272/1/tests/util/test_file_parser.py@298 PS1, Line 298: return '\n'.join(lines) > Are you sure this is enough? I try scenario from the JIRA and it still show Thanks for checking this Riza! Could you let me know what you changed in testdata/workloads/functional-query/queries/QueryTest/decimal-exprs.test? If you are changing the expected results in the subsection of ERRORS, I recall there will still be two additional newlines in the subsection of ERRORS even after this patch. This patch only removes one newline. But I will double-check it. This patch was originally intended for removing the additional newline in the subsection of RESULTS since that additional newline would fail the verification of the updated subsection of RESULTS. And you are correct. After this patch, that code comment ("The inverse of split_section_lines().") no longer holds. We could probably try removing additional newlines until there is only one left in the end of last line as you suggested. -- To view, visit http://gerrit.cloudera.org:8080/20272 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic7668a437267bd76afecba8f87ead32d82580414 Gerrit-Change-Number: 20272 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Fri, 28 Jul 2023 01:14:22 +0000 Gerrit-HasComments: Yes
