From 6ce3f466ccd38235093c2317d73fbef8c4b7f0ae Mon Sep 17 00:00:00 2001 From: Tue Ly Date: Fri, 21 Feb 2025 15:23:43 +0000 Subject: [PATCH 1/6] [libc] Temporarily disable LlvmLibcFileTest.WriteOnly in libc.test.src.__support.File.file_test.__hermetic__ due to precommit bots's consistent failures. --- libc/test/src/__support/File/file_test.cpp | 77 +++++++++++----------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/libc/test/src/__support/File/file_test.cpp b/libc/test/src/__support/File/file_test.cpp index b3c9f2ba49bce..9c634ca07a5d0 100644 --- a/libc/test/src/__support/File/file_test.cpp +++ b/libc/test/src/__support/File/file_test.cpp @@ -113,43 +113,46 @@ StringFile *new_string_file(char *buffer, size_t buflen, int bufmode, LIBC_NAMESPACE::File::mode_flags(mode)); } -TEST(LlvmLibcFileTest, WriteOnly) { - const char data[] = "hello, file"; - constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2; - char file_buffer[FILE_BUFFER_SIZE]; - StringFile *f = - new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w"); - - ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); - EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream - ASSERT_EQ(f->flush(), 0); - EXPECT_EQ(f->get_pos(), sizeof(data)); // Data should now be available - EXPECT_STREQ(f->get_str(), data); - - f->reset(); - ASSERT_EQ(f->get_pos(), size_t(0)); - ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); - EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream - // The second write should trigger a buffer flush. - ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); - EXPECT_GE(f->get_pos(), size_t(0)); - ASSERT_EQ(f->flush(), 0); - EXPECT_EQ(f->get_pos(), 2 * sizeof(data)); - MemoryView src1("hello, file\0hello, file", sizeof(data) * 2), - dst1(f->get_str(), sizeof(data) * 2); - EXPECT_MEM_EQ(src1, dst1); - - char read_data[sizeof(data)]; - { - // This is not a readable file. - auto result = f->read(read_data, sizeof(data)); - EXPECT_EQ(result.value, size_t(0)); - EXPECT_TRUE(f->error()); - EXPECT_TRUE(result.has_error()); - } - - ASSERT_EQ(f->close(), 0); -} +// TODO: Investigate the precommit bots' failures of this test and re-enable it. +// https://github.com/llvm/llvm-project/issues/128185. +// +// TEST(LlvmLibcFileTest, WriteOnly) { +// const char data[] = "hello, file"; +// constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2; +// char file_buffer[FILE_BUFFER_SIZE]; +// StringFile *f = +// new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w"); + +// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); +// EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream +// ASSERT_EQ(f->flush(), 0); +// EXPECT_EQ(f->get_pos(), sizeof(data)); // Data should now be available +// EXPECT_STREQ(f->get_str(), data); + +// f->reset(); +// ASSERT_EQ(f->get_pos(), size_t(0)); +// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); +// EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream +// // The second write should trigger a buffer flush. +// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); +// EXPECT_GE(f->get_pos(), size_t(0)); +// ASSERT_EQ(f->flush(), 0); +// EXPECT_EQ(f->get_pos(), 2 * sizeof(data)); +// MemoryView src1("hello, file\0hello, file", sizeof(data) * 2), +// dst1(f->get_str(), sizeof(data) * 2); +// EXPECT_MEM_EQ(src1, dst1); + +// char read_data[sizeof(data)]; +// { +// // This is not a readable file. +// auto result = f->read(read_data, sizeof(data)); +// EXPECT_EQ(result.value, size_t(0)); +// EXPECT_TRUE(f->error()); +// EXPECT_TRUE(result.has_error()); +// } + +// ASSERT_EQ(f->close(), 0); +// } TEST(LlvmLibcFileTest, WriteLineBuffered) { const char data[] = "hello\n file"; From 6c724748be090cafd894a9d8b5178191e61ef503 Mon Sep 17 00:00:00 2001 From: Tue Ly Date: Fri, 21 Feb 2025 15:32:40 +0000 Subject: [PATCH 2/6] Apply comment. --- libc/test/src/__support/File/file_test.cpp | 77 +++++++++++----------- 1 file changed, 39 insertions(+), 38 deletions(-) diff --git a/libc/test/src/__support/File/file_test.cpp b/libc/test/src/__support/File/file_test.cpp index 9c634ca07a5d0..cf01a78ef6423 100644 --- a/libc/test/src/__support/File/file_test.cpp +++ b/libc/test/src/__support/File/file_test.cpp @@ -115,44 +115,45 @@ StringFile *new_string_file(char *buffer, size_t buflen, int bufmode, // TODO: Investigate the precommit bots' failures of this test and re-enable it. // https://github.com/llvm/llvm-project/issues/128185. -// -// TEST(LlvmLibcFileTest, WriteOnly) { -// const char data[] = "hello, file"; -// constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2; -// char file_buffer[FILE_BUFFER_SIZE]; -// StringFile *f = -// new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w"); - -// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); -// EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream -// ASSERT_EQ(f->flush(), 0); -// EXPECT_EQ(f->get_pos(), sizeof(data)); // Data should now be available -// EXPECT_STREQ(f->get_str(), data); - -// f->reset(); -// ASSERT_EQ(f->get_pos(), size_t(0)); -// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); -// EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream -// // The second write should trigger a buffer flush. -// ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); -// EXPECT_GE(f->get_pos(), size_t(0)); -// ASSERT_EQ(f->flush(), 0); -// EXPECT_EQ(f->get_pos(), 2 * sizeof(data)); -// MemoryView src1("hello, file\0hello, file", sizeof(data) * 2), -// dst1(f->get_str(), sizeof(data) * 2); -// EXPECT_MEM_EQ(src1, dst1); - -// char read_data[sizeof(data)]; -// { -// // This is not a readable file. -// auto result = f->read(read_data, sizeof(data)); -// EXPECT_EQ(result.value, size_t(0)); -// EXPECT_TRUE(f->error()); -// EXPECT_TRUE(result.has_error()); -// } - -// ASSERT_EQ(f->close(), 0); -// } +#if 0 +TEST(LlvmLibcFileTest, WriteOnly) { + const char data[] = "hello, file"; + constexpr size_t FILE_BUFFER_SIZE = sizeof(data) * 3 / 2; + char file_buffer[FILE_BUFFER_SIZE]; + StringFile *f = + new_string_file(file_buffer, FILE_BUFFER_SIZE, _IOFBF, false, "w"); + + ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); + EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream + ASSERT_EQ(f->flush(), 0); + EXPECT_EQ(f->get_pos(), sizeof(data)); // Data should now be available + EXPECT_STREQ(f->get_str(), data); + + f->reset(); + ASSERT_EQ(f->get_pos(), size_t(0)); + ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); + EXPECT_EQ(f->get_pos(), size_t(0)); // Data is buffered in the file stream + // The second write should trigger a buffer flush. + ASSERT_EQ(sizeof(data), f->write(data, sizeof(data)).value); + EXPECT_GE(f->get_pos(), size_t(0)); + ASSERT_EQ(f->flush(), 0); + EXPECT_EQ(f->get_pos(), 2 * sizeof(data)); + MemoryView src1("hello, file\0hello, file", sizeof(data) * 2), + dst1(f->get_str(), sizeof(data) * 2); + EXPECT_MEM_EQ(src1, dst1); + + char read_data[sizeof(data)]; + { + // This is not a readable file. + auto result = f->read(read_data, sizeof(data)); + EXPECT_EQ(result.value, size_t(0)); + EXPECT_TRUE(f->error()); + EXPECT_TRUE(result.has_error()); + } + + ASSERT_EQ(f->close(), 0); +} +#endif TEST(LlvmLibcFileTest, WriteLineBuffered) { const char data[] = "hello\n file"; From f55e5f6528e36a4e0a86c21d886e6f7b92293ef0 Mon Sep 17 00:00:00 2001 From: Tue Ly Date: Fri, 21 Feb 2025 16:11:00 +0000 Subject: [PATCH 3/6] New file test failure. --- libc/test/src/__support/File/platform_file_test.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libc/test/src/__support/File/platform_file_test.cpp b/libc/test/src/__support/File/platform_file_test.cpp index 6b2be2a149329..b232544317b18 100644 --- a/libc/test/src/__support/File/platform_file_test.cpp +++ b/libc/test/src/__support/File/platform_file_test.cpp @@ -20,6 +20,9 @@ LIBC_INLINE File *openfile(const char *file_name, const char *mode) { return error_or_file.has_value() ? error_or_file.value() : nullptr; } +// TODO: Investigate the precommit bots' failures of this test and re-enable it. +// https://github.com/llvm/llvm-project/issues/128185. +#if 0 TEST(LlvmLibcPlatformFileTest, CreateWriteCloseAndReadBack) { constexpr char FILENAME[] = "testdata/create_write_close_and_readback.test"; File *file = openfile(FILENAME, "w"); @@ -40,6 +43,7 @@ TEST(LlvmLibcPlatformFileTest, CreateWriteCloseAndReadBack) { ASSERT_EQ(file->close(), 0); } +#endif TEST(LlvmLibcPlatformFileTest, CreateWriteSeekAndReadBack) { constexpr char FILENAME[] = "testdata/create_write_seek_and_readback.test"; From 101ba37fb8d7706f31cdb448fd809dfe9651fdb5 Mon Sep 17 00:00:00 2001 From: Tue Ly Date: Fri, 21 Feb 2025 20:15:31 +0000 Subject: [PATCH 4/6] Try to set the test hermetic only in full build mode. --- libc/test/src/__support/File/CMakeLists.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/libc/test/src/__support/File/CMakeLists.txt b/libc/test/src/__support/File/CMakeLists.txt index a11f52978f35f..b1ebfd71be7ae 100644 --- a/libc/test/src/__support/File/CMakeLists.txt +++ b/libc/test/src/__support/File/CMakeLists.txt @@ -5,8 +5,17 @@ if(NOT (TARGET libc.src.__support.threads.mutex) OR LIBC_TARGET_OS_IS_GPU) return() endif() +# TODO: Investigate the precommit bots' failures of this test and re-enable it. +# https://github.com/llvm/llvm-project/issues/128185. +set(file_test_hermetic_only "") +if(LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND LIBC_TARGET_OS_IS_LINUX AND + LLVM_LIBC_FULL_BUILD) + set(file_test_hermetic_only "HERMETIC_TEST_ONLY") +endif() + add_libc_test( file_test + ${file_test_hermetic_only} SUITE libc-support-tests SRCS @@ -23,6 +32,7 @@ add_libc_test( add_libc_test( platform_file_test + ${file_test_hermetic_only} SUITE libc-support-tests SRCS From 90b12459069f1189770b3c4710158993ef91a385 Mon Sep 17 00:00:00 2001 From: Tue Ly Date: Fri, 21 Feb 2025 21:12:59 +0000 Subject: [PATCH 5/6] Remove x86_64 check because the precommit failed in aarch64. --- libc/test/src/__support/File/CMakeLists.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libc/test/src/__support/File/CMakeLists.txt b/libc/test/src/__support/File/CMakeLists.txt index b1ebfd71be7ae..33e8e9ecc282f 100644 --- a/libc/test/src/__support/File/CMakeLists.txt +++ b/libc/test/src/__support/File/CMakeLists.txt @@ -8,8 +8,7 @@ endif() # TODO: Investigate the precommit bots' failures of this test and re-enable it. # https://github.com/llvm/llvm-project/issues/128185. set(file_test_hermetic_only "") -if(LIBC_TARGET_ARCHITECTURE_IS_X86_64 AND LIBC_TARGET_OS_IS_LINUX AND - LLVM_LIBC_FULL_BUILD) +if(LIBC_TARGET_OS_IS_LINUX AND LLVM_LIBC_FULL_BUILD) set(file_test_hermetic_only "HERMETIC_TEST_ONLY") endif() From 82283423b9c16212f5cc153946a40cb06a85e57b Mon Sep 17 00:00:00 2001 From: Tue Ly Date: Fri, 21 Feb 2025 21:32:54 +0000 Subject: [PATCH 6/6] Change to UNIT_TEST_ONLY. --- libc/test/src/__support/File/CMakeLists.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libc/test/src/__support/File/CMakeLists.txt b/libc/test/src/__support/File/CMakeLists.txt index 33e8e9ecc282f..57c37aa7dd220 100644 --- a/libc/test/src/__support/File/CMakeLists.txt +++ b/libc/test/src/__support/File/CMakeLists.txt @@ -7,14 +7,14 @@ endif() # TODO: Investigate the precommit bots' failures of this test and re-enable it. # https://github.com/llvm/llvm-project/issues/128185. -set(file_test_hermetic_only "") +set(file_test_unit_only "") if(LIBC_TARGET_OS_IS_LINUX AND LLVM_LIBC_FULL_BUILD) - set(file_test_hermetic_only "HERMETIC_TEST_ONLY") + set(file_test_unit_only "UNIT_TEST_ONLY") endif() add_libc_test( file_test - ${file_test_hermetic_only} + ${file_test_unit_only} SUITE libc-support-tests SRCS @@ -31,7 +31,7 @@ add_libc_test( add_libc_test( platform_file_test - ${file_test_hermetic_only} + ${file_test_unit_only} SUITE libc-support-tests SRCS