Skip to content

[libc++][format] Fixed println.blank_line.sh.cpp test on llvm-clang-win-x-* configurations #88011

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

Conversation

H-G-Hristov
Copy link
Contributor

@H-G-Hristov H-G-Hristov commented Apr 8, 2024

Fix for issue: #87277 (comment)

The test fails on the windows to linux cross builders. The proposed resolution is to print some text. The issue is possibly due to the original test outputting a single \n character.

@H-G-Hristov H-G-Hristov requested a review from a team as a code owner April 8, 2024 16:31
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Apr 8, 2024
@llvmbot
Copy link
Member

llvmbot commented Apr 8, 2024

@llvm/pr-subscribers-libcxx

Author: Hristo Hristov (H-G-Hristov)

Changes

Fix for issue: #87277 (comment)


Full diff: https://github.com/llvm/llvm-project/pull/88011.diff

1 Files Affected:

  • (modified) libcxx/test/std/input.output/iostream.format/print.fun/println.blank_line.sh.cpp (+3-1)
diff --git a/libcxx/test/std/input.output/iostream.format/print.fun/println.blank_line.sh.cpp b/libcxx/test/std/input.output/iostream.format/print.fun/println.blank_line.sh.cpp
index 93c3563d501b20..a262c287108a4c 100644
--- a/libcxx/test/std/input.output/iostream.format/print.fun/println.blank_line.sh.cpp
+++ b/libcxx/test/std/input.output/iostream.format/print.fun/println.blank_line.sh.cpp
@@ -35,13 +35,15 @@
 
 // FILE_DEPENDENCIES: echo.sh
 // RUN: %{build}
-// RUN: %{exec} bash echo.sh -ne "\n" > %t.expected
+// RUN: %{exec} bash echo.sh -ne "println blank line test: \n" > %t.expected
 // RUN: %{exec} "%t.exe" > %t.actual
 // RUN: diff -u %t.actual %t.expected
 
 #include <print>
 
 int main(int, char**) {
+  // On some configurations the `diff -u` test fails if we print a single blank line character `\n`, so we print some text first.
+  std::print("println blank line test: ");
   std::println();
 
   return 0;

Copy link
Member

@mordante mordante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the quick fix! LGTM!
Please give @vvereschaka some time to respond and possibly test the fix prior to landing.

@H-G-Hristov
Copy link
Contributor Author

Thanks for the quick fix! LGTM! Please give @vvereschaka some time to respond and possibly test the fix prior to landing.

@mordante Thank you!
@vvereschaka If you approve this fix. Please feel free to merge it.

@vvereschaka
Copy link
Contributor

@H-G-Hristov , @mordante
thank you for the fix. I'll test it during the next hour.

@vvereschaka
Copy link
Contributor

The local test has passed successfully. Thanks once again.
I'm going to merge the fix.

@vvereschaka vvereschaka merged commit ea2392e into llvm:main Apr 8, 2024
@H-G-Hristov H-G-Hristov deleted the hgh/libcxx/fix-println.blank_line.sh.cpp-test branch April 13, 2024 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants