[PATCH] D52638: [XRay] Fix fdr-thread-order.cc when current directory contains fdr-thread-order.cc

[Fangrui Song via Phabricator via llvm-commits]

MaskRay marked an inline comment as done.
MaskRay added inline comments.


================
Comment at: test/xray/TestCases/Posix/fdr-thread-order.cc:2
+// RUN: rm -rf %t && mkdir %t
+// RUN: %clangxx_xray -g -std=c++11 %s -o %t.exe
+// RUN: XRAY_OPTIONS="patch_premain=false \
----------------
dberris wrote:
> I'm not a fan of the `.exe` extension here -- consider `-bin` instead?
I am not a fan of Windows but chose `.exe` because I've seen `%t.exe` used in llvm-readobj gold lld tests. They are also used in `compiler-rt/test/asan/TestCases/{Posix,Linux,Darwin}`. In ELF, such executables are called `ET_EXEC` before PIE era. The `.exe` suffix should not be too exotic.


================
Comment at: test/xray/TestCases/Posix/fdr-thread-order.cc:12
-// RUN:    FileCheck %s --check-prefix TRACE
-// RUN: rm fdr-thread-order.*
 // FIXME: Make llvm-xray work on non-x86_64 as well.
----------------
dberris wrote:
> Unfortunately, doing this makes it really easy to clog up the filesystem with log files upon failure (which I learned the hard way while working on these). The suggestion has been to re-create the sequence by hand if we want to debug, rather than to keep them lying around when the tests fail.
> 
> Thoughts?
If the tests use unique directory names to isolate, it shouldn't clog up the filesystem.

Are sizes of log files an issue? If not, I think not cleaning the temporary files should be favored, but right now tests put `xray-log.*` in the current directory, which easily lead to conflicts.

I've added back `rm -rf %t` here. We may discuss this later. If we reach a consensus, I can create a refactor revision for other tests as well, if needed.


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D52638