Skip to content

Commit e04fe9b

Browse files
authored
Fix a flaky test (o3de#17567)
FileDescriptorCapturer_DoesNotDeadlock_WhenMoreThanPipeSizeContent_IsCaptured The above test was succeeding when run directly (even with repeat 1000), but was failing every single time for me when run in parallel by pytest. My assumption here is that stdout was being spammed somehow by pytest or other threads running at the same time. To fix this, I made it write to a temp file in a auto delete temp folder instead. After this change it no longer failed during local ci_build pytest runs. Signed-off-by: Nicholas Lawson <[email protected]>
1 parent 40cde77 commit e04fe9b

File tree

1 file changed

+34
-12
lines changed

1 file changed

+34
-12
lines changed

Code/Framework/AzCore/Tests/SystemFileTest.cpp

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -253,32 +253,54 @@ namespace UnitTest
253253

254254
TEST_F(SystemFileTest, FileDescriptorCapturer_DoesNotDeadlock_WhenMoreThanPipeSizeContent_IsCaptured)
255255
{
256-
AZStd::string stdoutData;
257-
auto StoreStdout = [&stdoutData](AZStd::span<const AZStd::byte> capturedBytes)
256+
using namespace AZ::IO;
257+
258+
AZStd::string capturedData;
259+
auto StoreFromFile = [&capturedData](AZStd::span<const AZStd::byte> capturedBytes)
258260
{
259261
AZStd::string_view capturedStrView(reinterpret_cast<const char*>(capturedBytes.data()), capturedBytes.size());
260-
stdoutData += capturedStrView;
262+
capturedData += capturedStrView;
261263
};
262264

263265
// The +1 to make sure the value isn't a power of 2, to try to catch any edge cases
264266
// with reading from a buffered pipe
265267
constexpr size_t charCountToWrite = AZ::IO::FileDescriptorCapturer::DefaultPipeSize * 2 + 1;
266-
stdoutData.reserve(charCountToWrite);
268+
capturedData.reserve(charCountToWrite);
267269

268-
// file descriptor 1 is stdout
269-
constexpr int StdoutDescriptor = 1;
270-
AZ::IO::FileDescriptorCapturer capturer(StdoutDescriptor);
271-
capturer.Start(StoreStdout);
272-
// Capture twice the DefaultPipeSize amount of bytes
270+
// while it may be tempting to use stdout here, other processes and threads might be running
271+
// in the same test run and also outputting to stdout. This would cause an intermittent failure.
272+
// Instead, write to a temp file.
273+
AZ::Test::ScopedAutoTempDirectory tempDir;
274+
auto srcFile = tempDir.Resolve("SystemFileTest_Source.txt");
275+
int sourceFd = PosixInternal::Open(srcFile.c_str(),
276+
PosixInternal::OpenFlags::Create | PosixInternal::OpenFlags::ReadWrite | PosixInternal::OpenFlags::Truncate,
277+
PosixInternal::PermissionModeFlags::Read | PosixInternal::PermissionModeFlags::Write);
278+
ASSERT_NE(sourceFd, -1);
279+
AZ::IO::FileDescriptorCapturer capturer(sourceFd);
280+
281+
capturer.Start(StoreFromFile);
282+
const char* dataToWrite = "a";
273283
for (size_t i = 0; i < charCountToWrite; ++i)
274284
{
275-
fputc('a', stdout);
285+
// this should cause the write function to fill up any buffer and then block if the pipe is full
286+
// until the capturer reads from it.
287+
// filling the pipe should NOT cause blocking here, since the capturer reads on a different thread.
288+
PosixInternal::Write(sourceFd, dataToWrite, 1);
276289
}
277-
fflush(stdout);
290+
PosixInternal::Close(sourceFd);
278291
capturer.Stop();
279292

280293
const AZStd::string expectedValue(charCountToWrite, 'a');
281-
EXPECT_EQ(expectedValue, stdoutData);
294+
295+
// if the lengths are not equal, we DROPPED or invented data.
296+
EXPECT_EQ(expectedValue.size(), capturedData.size()) << "FileDescriptorCapturer dropped or invented some of the data.";
297+
298+
if (expectedValue.size() == capturedData.size())
299+
{
300+
// if the lengths are equal, but the strings are different, then the data is corrupt in some way (full of the
301+
// wrong character, or nulls, or something).
302+
EXPECT_TRUE(expectedValue == capturedData) << "FileDescriptorCapturer corrupted some of the data";
303+
}
282304
}
283305

284306
TEST_F(SystemFileTest, GetStdout_ReturnsHandle_ThatCanWriteToStdout_Succeeds)

0 commit comments

Comments
 (0)