[flang-commits] [flang] [NFC][flang] Fix execute_command_line test for odd environments (PR #117714)

[David Truby via flang-commits]

https://github.com/DavidTruby updated https://github.com/llvm/llvm-project/pull/117714

>From de7f99739dd3cd97b729c353f7b66107ba4148ce Mon Sep 17 00:00:00 2001
From: David Truby <david at truby.dev>
Date: Tue, 26 Nov 2024 13:48:41 +0000
Subject: [PATCH 1/4] [NFC][flang] Fix execute_command_line test for odd
 environments

One of the execute_command_line tests currently runs `cat` on an invalid
file and checks its return value, but since we don't control `cat` or
the user's path, the return value might not be reliably stable on a
per-platform basis. For example, if `git` is installed on Windows in
certain configurations it adds a directory to the path containing a
`cat` with a different set of error codes to the default Windows one.

This patch changes the test to use the `not` binary built by LLVM for
testing purposes, which should always return 1 on any platform
regardless of the user's environment.
---
 flang/unittests/Runtime/CommandTest.cpp | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index b0c43ba01d8f33..34a6296d9cdf57 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -13,6 +13,8 @@
 #include "flang/Runtime/execute.h"
 #include "flang/Runtime/extensions.h"
 #include "flang/Runtime/main.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
 #include <cstddef>
 #include <cstdlib>
 
@@ -340,7 +342,11 @@ TEST_F(ZeroArguments, ECLValidCommandStatusSetSync) {
 }
 
 TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
-  OwningPtr<Descriptor> command{CharDescriptor("cat GeneralErrorCommand")};
+  llvm::SmallString<64> cmd;
+  if (std::error_code ec = llvm::sys::fs::current_path(cmd))
+    FAIL() << "Failed to obtain the current working directory";
+  llvm::sys::path::append(cmd, "bin", "not");
+  OwningPtr<Descriptor> command{CharDescriptor(cmd.data())};
   bool wait{true};
   OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
   OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};
@@ -348,16 +354,14 @@ TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
 
   RTNAME(ExecuteCommandLine)
   (*command.get(), wait, exitStat.get(), cmdStat.get(), cmdMsg.get());
-#if defined(_WIN32)
   CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
+#if defined(_WIN32)
   CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
   CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXXXX");
 #elif defined(_AIX)
-  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 2);
   CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 6);
   CheckDescriptorEqStr(cmdMsg.get(), "Invalid command lineXXXXXXXXX");
 #else
-  CheckDescriptorEqInt<std::int64_t>(exitStat.get(), 1);
   CheckDescriptorEqInt<std::int64_t>(cmdStat.get(), 3);
   CheckDescriptorEqStr(cmdMsg.get(), "Command line execution failed");
 #endif

>From 913f011a364695c7e51c0b3445cfde301ca23313 Mon Sep 17 00:00:00 2001
From: David Truby <david at truby.dev>
Date: Tue, 26 Nov 2024 14:19:19 +0000
Subject: [PATCH 2/4] use LLVM_TOOLS_BINARY_DIR

The not binary will always be placed in this directory.
---
 flang/unittests/Runtime/CMakeLists.txt  | 2 ++
 flang/unittests/Runtime/CommandTest.cpp | 5 ++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/flang/unittests/Runtime/CMakeLists.txt b/flang/unittests/Runtime/CMakeLists.txt
index 2c3f8c1a9e9ac8..0a254accd02e79 100644
--- a/flang/unittests/Runtime/CMakeLists.txt
+++ b/flang/unittests/Runtime/CMakeLists.txt
@@ -36,4 +36,6 @@ target_link_libraries(FlangRuntimeTests
   FortranRuntime
 )
 
+target_compile_definitions(FlangRuntimeTests PRIVATE LLVM_TOOLS_BINARY_DIR="${LLVM_TOOLS_BINARY_DIR}")
+
 add_subdirectory(CUDA)
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 34a6296d9cdf57..fdb254b9d96a9f 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -343,9 +343,8 @@ TEST_F(ZeroArguments, ECLValidCommandStatusSetSync) {
 
 TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
   llvm::SmallString<64> cmd;
-  if (std::error_code ec = llvm::sys::fs::current_path(cmd))
-    FAIL() << "Failed to obtain the current working directory";
-  llvm::sys::path::append(cmd, "bin", "not");
+  llvm::sys::path::native(LLVM_TOOLS_BINARY_DIR, cmd);
+  llvm::sys::path::append(cmd, "not");
   OwningPtr<Descriptor> command{CharDescriptor(cmd.data())};
   bool wait{true};
   OwningPtr<Descriptor> exitStat{IntDescriptor(404)};

>From ae4c677051fcf644505549bdaa4b5cd1fcb48bdb Mon Sep 17 00:00:00 2001
From: David Truby <david at truby.dev>
Date: Tue, 26 Nov 2024 14:24:21 +0000
Subject: [PATCH 3/4] Remove Filesystem.h include

---
 flang/unittests/Runtime/CommandTest.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index fdb254b9d96a9f..22eccfee60b1aa 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -13,7 +13,6 @@
 #include "flang/Runtime/execute.h"
 #include "flang/Runtime/extensions.h"
 #include "flang/Runtime/main.h"
-#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include <cstddef>
 #include <cstdlib>

>From a812045562ee6c00f5cbca7ee2b8b2858ec6b81b Mon Sep 17 00:00:00 2001
From: David Truby <david at truby.dev>
Date: Tue, 26 Nov 2024 14:52:09 +0000
Subject: [PATCH 4/4] Use direct path to the `not` exe from CMake

---
 flang/unittests/Runtime/CMakeLists.txt  | 2 +-
 flang/unittests/Runtime/CommandTest.cpp | 6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/flang/unittests/Runtime/CMakeLists.txt b/flang/unittests/Runtime/CMakeLists.txt
index 0a254accd02e79..179e439917ff23 100644
--- a/flang/unittests/Runtime/CMakeLists.txt
+++ b/flang/unittests/Runtime/CMakeLists.txt
@@ -36,6 +36,6 @@ target_link_libraries(FlangRuntimeTests
   FortranRuntime
 )
 
-target_compile_definitions(FlangRuntimeTests PRIVATE LLVM_TOOLS_BINARY_DIR="${LLVM_TOOLS_BINARY_DIR}")
+target_compile_definitions(FlangRuntimeTests PRIVATE NOT_EXE="$<TARGET_FILE:not>")
 
 add_subdirectory(CUDA)
diff --git a/flang/unittests/Runtime/CommandTest.cpp b/flang/unittests/Runtime/CommandTest.cpp
index 22eccfee60b1aa..05287d80e14f58 100644
--- a/flang/unittests/Runtime/CommandTest.cpp
+++ b/flang/unittests/Runtime/CommandTest.cpp
@@ -13,7 +13,6 @@
 #include "flang/Runtime/execute.h"
 #include "flang/Runtime/extensions.h"
 #include "flang/Runtime/main.h"
-#include "llvm/Support/Path.h"
 #include <cstddef>
 #include <cstdlib>
 
@@ -341,10 +340,7 @@ TEST_F(ZeroArguments, ECLValidCommandStatusSetSync) {
 }
 
 TEST_F(ZeroArguments, ECLGeneralErrorCommandErrorSync) {
-  llvm::SmallString<64> cmd;
-  llvm::sys::path::native(LLVM_TOOLS_BINARY_DIR, cmd);
-  llvm::sys::path::append(cmd, "not");
-  OwningPtr<Descriptor> command{CharDescriptor(cmd.data())};
+  OwningPtr<Descriptor> command{CharDescriptor(NOT_EXE)};
   bool wait{true};
   OwningPtr<Descriptor> exitStat{IntDescriptor(404)};
   OwningPtr<Descriptor> cmdStat{IntDescriptor(202)};