[Lldb-commits] [PATCH] D67230: Remove call to obsolete gethostbyname, using getaddrinfo

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

This code is trying too hard and failing. Either the result of gethostname() is 
canonical or it is not. If it is not, then trying to canonicalize it is – for 
various reasons – a lost cause. For example, a given machine might have 
multiple network interfaces with multiple addresses per interface, each with a 
different canonical name. Separably, the result of 
`HostInfoPosix::GetHostname()` and latency thereof shouldn't depend on whether 
networking is up or down or what network the machine happened to be attached to 
at any given moment (like a laptop that travels between work and home).


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67230/new/

https://reviews.llvm.org/D67230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D67230: Remove call to obsolete gethostbyname, using getaddrinfo

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Great. See: r371596


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67230/new/

https://reviews.llvm.org/D67230



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59991: [Linux/x86] Fix writing of non-gpr registers on newer processors

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki accepted this revision.
davezarzycki added a comment.

I've verified that this fixes my Skylake-SP (Xeon 8168) workstation. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59991/new/

https://reviews.llvm.org/D59991



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D59991: [Linux/x86] Fix writing of non-gpr registers on newer processors

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Can we cherry-pick this into a release branch? Skylake (and newer) CPUs are far 
from rare these days, especially on cloud hosting providers.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59991/new/

https://reviews.llvm.org/D59991



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D82259: Deprecate error prone temporary directory APIs

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki updated this revision to Diff 272661.
davezarzycki added a comment.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

Moved clang specific changes to: https://reviews.llvm.org/D82362


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82259/new/

https://reviews.llvm.org/D82259

Files:
  lldb/source/Core/ModuleList.cpp
  llvm/include/llvm/Support/FileSystem.h
  llvm/include/llvm/Support/Path.h
  llvm/lib/Support/Path.cpp

Index: llvm/lib/Support/Path.cpp
===
--- llvm/lib/Support/Path.cpp
+++ llvm/lib/Support/Path.cpp
@@ -164,6 +164,41 @@
   FS_Name
 };
 
+// Avoid a deprecation warning by moving the createUniquePath body here.
+static void _createUniquePath(const Twine &Model,
+  SmallVectorImpl &ResultPath,
+  bool MakeAbsolute) {
+  SmallString<128> ModelStorage;
+  Model.toVector(ModelStorage);
+
+  if (MakeAbsolute) {
+// Make model absolute by prepending a temp directory if it's not already.
+if (!sys::path::is_absolute(Twine(ModelStorage))) {
+  SmallString<128> TDir;
+#ifdef __clang__
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wdeprecated-declarations"
+#endif
+  sys::path::system_temp_directory(true, TDir);
+#ifdef __clang__
+#pragma clang diagnostic pop
+#endif
+  sys::path::append(TDir, Twine(ModelStorage));
+  ModelStorage.swap(TDir);
+}
+  }
+
+  ResultPath = ModelStorage;
+  ResultPath.push_back(0);
+  ResultPath.pop_back();
+
+  // Replace '%' with random chars.
+  for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
+if (ModelStorage[i] == '%')
+  ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
+  }
+}
+
 static std::error_code
 createUniqueEntity(const Twine &Model, int &ResultFD,
SmallVectorImpl &ResultPath, bool MakeAbsolute,
@@ -176,7 +211,7 @@
   // Checking which is racy, so we try a number of times, then give up.
   std::error_code EC;
   for (int Retries = 128; Retries > 0; --Retries) {
-sys::fs::createUniquePath(Model, ResultPath, MakeAbsolute);
+_createUniquePath(Model, ResultPath, MakeAbsolute);
 // Try to open + create the file.
 switch (Type) {
 case FS_File: {
@@ -778,28 +813,7 @@
 
 void createUniquePath(const Twine &Model, SmallVectorImpl &ResultPath,
   bool MakeAbsolute) {
-  SmallString<128> ModelStorage;
-  Model.toVector(ModelStorage);
-
-  if (MakeAbsolute) {
-// Make model absolute by prepending a temp directory if it's not already.
-if (!sys::path::is_absolute(Twine(ModelStorage))) {
-  SmallString<128> TDir;
-  sys::path::system_temp_directory(true, TDir);
-  sys::path::append(TDir, Twine(ModelStorage));
-  ModelStorage.swap(TDir);
-}
-  }
-
-  ResultPath = ModelStorage;
-  ResultPath.push_back(0);
-  ResultPath.pop_back();
-
-  // Replace '%' with random chars.
-  for (unsigned i = 0, e = ModelStorage.size(); i != e; ++i) {
-if (ModelStorage[i] == '%')
-  ResultPath[i] = "0123456789abcdef"[sys::Process::GetRandomNumber() & 15];
-  }
+  return _createUniquePath(Model, ResultPath, MakeAbsolute);
 }
 
 std::error_code createUniqueFile(const Twine &Model, int &ResultFd,
Index: llvm/include/llvm/Support/Path.h
===
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ -363,7 +363,10 @@
 /// (e.g., TEMP on Windows, TMPDIR on *nix) to specify a temporary directory.
 ///
 /// @param result Holds the resulting path name.
-void system_temp_directory(bool erasedOnReboot, SmallVectorImpl &result);
+LLVM_ATTRIBUTE_DEPRECATED(
+void system_temp_directory(bool erasedOnReboot,
+   SmallVectorImpl &result),
+"Only meant for debugging due to trivial security problems");
 
 /// Get the user's home directory.
 ///
Index: llvm/include/llvm/Support/FileSystem.h
===
--- llvm/include/llvm/Support/FileSystem.h
+++ llvm/include/llvm/Support/FileSystem.h
@@ -800,8 +800,10 @@
 /// @param Model Name to base unique path off of.
 /// @param ResultPath Set to the file's path.
 /// @param MakeAbsolute Whether to use the system temp directory.
-void createUniquePath(const Twine &Model, SmallVectorImpl &ResultPath,
-  bool MakeAbsolute);
+LLVM_ATTRIBUTE_DEPRECATED(
+void createUniquePath(const Twine &Model, SmallVectorImpl &ResultPath,
+  bool MakeAbsolute),
+"Use createUniqueFile() or createUniqueDirectory()");
 
 /// Create a uniquely named file.
 ///
Index: lldb/source/Core/ModuleList.cpp
===
--- lldb/source/Core/ModuleList.cpp
+++ lldb/source/Core/ModuleList.cpp
@@ -82,8 +82,8 @@

[Lldb-commits] [PATCH] D82259: Deprecate error prone temporary directory APIs

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki abandoned this revision.
davezarzycki added a comment.

Now that my core concern is addressed (moving clang's default module cache out 
of /tmp), I don't have the time to push for this deprecation. Sorry.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82259/new/

https://reviews.llvm.org/D82259



___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D86497: [lldb] Add reproducer verifier

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Hello. I have an auto-bisecting multi-stage bot that has identified this change 
as breaking release (without assertions) testing on Fedora 33 x86-64. Can we 
get a quick fix or revert this change for now?

  FAIL: lldb-shell :: Reproducer/Functionalities/TestImageList.test (68782 of 
69683)
   TEST 'lldb-shell :: 
Reproducer/Functionalities/TestImageList.test' FAILED 
  Script:
  --
  : 'RUN: at line 6';   /tmp/_update_lc/r/bin/clang 
--target=specify-a-target-or-use-a-_host-substitution 
--target=x86_64-unknown-linux-gnu -pthread 
-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell
 
/home/dave/ro_s/lp/lldb/test/Shell/Reproducer/Functionalities/Inputs/stepping.c 
-g -o 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.out
  : 'RUN: at line 8';   rm -rf 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
  : 'RUN: at line 10';   echo "CAPTURE" >> 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
  : 'RUN: at line 11';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S 
/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init -x -b  --capture 
--capture-path 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.repro
 -o 'run' -o 'image list' -o 'reproducer generate' 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.out
 >> 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
 2>&1
  : 'RUN: at line 17';   echo "REPLAY" >> 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
  : 'RUN: at line 18';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S 
/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init -x -b --replay 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.repro
 >> 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
 2>&1
  : 'RUN: at line 20';   cat 
/tmp/_update_lc/r/tools/lldb/test/Reproducer/Functionalities/Output/TestImageList.test.tmp.txt
 | /tmp/_update_lc/r/bin/FileCheck 
/home/dave/ro_s/lp/lldb/test/Shell/Reproducer/Functionalities/TestImageList.test
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  clang-12: warning: argument unused during compilation: 
'-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell'
 [-Wunused-command-line-argument]
  
  --
  
  
  Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
  
  Failed Tests (1):
lldb-shell :: Reproducer/Functionalities/TestImageList.test
  
  
  Testing Time: 125.54s
Unsupported  : 10774
Passed   : 58806
Expectedly Failed:   102
Failed   : 1
  FAILED: CMakeFiles/check-all


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86497/new/

https://reviews.llvm.org/D86497

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

In D98179#2689079 , @lebedev.ri wrote:

> In D98179#2689075 , @mstorsjo wrote:
>
>> Something related to the time recording seems to fail intermittently on 
>> buildbots: https://lab.llvm.org/buildbot#builders/93/builds/2697
>
> I'll cut to the fix here: @davezarzycki is there any reason why the timing 
> data is stored in a CSV format, and not JSON?
> The fix is to switch to latter.

What? Weird. The file isn't CSV. It's just a number followed by a space and 
then everything until the newline is the file name. Nothing fancy.  Switching 
to JSON probably just means papering over the real problem which seems to be 
either that sometimes a test has an empty string for a name or that there is an 
edge case in python's number printing that can result in pure whitespace. We'd 
need to look at the timing data file to see why it's bogus (or corrupt).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D86497: [lldb] Add reproducer verifier

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Line 18 fails: `%lldb -x -b --replay %t.repro >> %t.txt 2>&1`

  error: reproducer replay failed:
  warning: home directory '/home/dave' not in VFS

I tried rebuilding in my home directory as opposed to /tmp and the error went 
away. Did you try building outside of your home directory?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86497/new/

https://reviews.llvm.org/D86497

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D86497: [lldb] Add reproducer verifier

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

And just to be clear, the source directory in my setup is in the home 
directory. My cron job / "bot" build just builds in /tmp.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86497/new/

https://reviews.llvm.org/D86497

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D88123: Add the ability to write 'target stop-hooks' in Python

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Hi Jim, this change broke my Fedora 33 Linux (x86) box. Do you think we can get 
a quick fix or revert this?

  FAIL: lldb-shell :: Commands/command-stop-hook-output.test (69796 of 70913)
   TEST 'lldb-shell :: 
Commands/command-stop-hook-output.test' FAILED 
  Script:
  --
  : 'RUN: at line 1';   /tmp/_update_lc/r/bin/clang 
--target=specify-a-target-or-use-a-_host-substitution 
--target=x86_64-unknown-linux-gnu -pthread 
-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell
 -g /home/dave/ro_s/lp/lldb/test/Shell/Commands/Inputs/main.c -o 
/tmp/_update_lc/r/tools/lldb/test/Commands/Output/command-stop-hook-output.test.tmp
  : 'RUN: at line 2';   /tmp/_update_lc/r/bin/lldb --no-lldbinit -S 
/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init 
/tmp/_update_lc/r/tools/lldb/test/Commands/Output/command-stop-hook-output.test.tmp
 -O 'command script import 
/home/dave/ro_s/lp/lldb/test/Shell/Commands/Inputs/stop_hook.py' -s 
/home/dave/ro_s/lp/lldb/test/Shell/Commands/command-stop-hook-output.test -o 
exit | /tmp/_update_lc/r/bin/FileCheck 
/home/dave/ro_s/lp/lldb/test/Shell/Commands/command-stop-hook-output.test
  --
  Exit Code: 1
  
  Command Output (stderr):
  --
  clang-12: warning: argument unused during compilation: 
'-fmodules-cache-path=/tmp/_update_lc/r/lldb-test-build.noindex/module-cache-clang/lldb-shell'
 [-Wunused-command-line-argument]
  error: module importing failed: This script interpreter does not support 
importing modules.
  
/home/dave/ro_s/lp/lldb/test/Shell/Commands/command-stop-hook-output.test:5:16: 
error: CHECK-LABEL: expected string not found in input
  # CHECK-LABEL: b main
 ^
  :1:1: note: scanning from here
  (lldb) command source -s 0 
'/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init'
  ^
  :1:8: note: possible intended match here
  (lldb) command source -s 0 
'/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init'
 ^
  
  Input file: 
  Check file: 
/home/dave/ro_s/lp/lldb/test/Shell/Commands/command-stop-hook-output.test
  
  -dump-input=help explains the following input dump.
  
  Input was:
  <<
 1: (lldb) command source -s 0 
'/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init'
  label:5'0 
X~
 error: no match found
  label:5'1?
   possible intended match
 2: Executing commands in 
'/tmp/_update_lc/r/tools/lldb/test/Shell/lit-lldb-init'.
  label:5'0 
~~
 3: (lldb) # LLDB init file for the LIT tests.
  label:5'0 ~~
 4: (lldb) settings set symbols.enable-external-lookup false
  label:5'0 
 5: (lldb) settings set plugin.process.gdb-remote.packet-timeout 60
  label:5'0 ~~~
 6: (lldb) settings set interpreter.echo-comment-commands false
  label:5'0 ~~~
 .
 .
 .
  >>
  
  --
  
  
  
  Failed Tests (1):
lldb-shell :: Commands/command-stop-hook-output.test
  
  
  Testing Time: 66.80s
Unsupported  : 10853
Passed   : 59957
Expectedly Failed:   102
Failed   : 1
  FAILED: CMakeFiles/check-all


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88123/new/

https://reviews.llvm.org/D88123

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki updated this revision to Diff 329613.
davezarzycki added a comment.

I believe I have addressed all of the feedback to date.

For people that care about easily identifying the failures from the previous 
testing run, that is now encoded via the sign of the test time (please remember 
that negative test times are impossible). For example:

3.14159 a-successful-test.txt
-2.71828 a-failed-test.txt

I've also implemented but commented out how one might use this to implement 
`--only-failures`. Feedback would be appreciated but I don't want to derail 
this change proposal on what should be an independent change.

Finally, python is not my native programming language, so thank you for helping 
me with idiomatic changes. What I'd really like help with though is figuring 
out a way to not pass the prior test timing data down as a new parameter to 
various methods. I tried to hang the data off the test suite data structure but 
that had brutal performance side effects. It seemed like python was hammering 
on the global interpreter lock despite the prior timing data being unused after 
the sorting phase. Weird. Help would be greatly appreciated. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

Files:
  libcxx/utils/libcxx/test/format.py
  libcxx/utils/libcxx/test/googlebenchmark.py
  lldb/test/API/lldbtest.py
  llvm/docs/CommandGuide/lit.rst
  llvm/test/Unit/lit.cfg.py
  llvm/utils/lit/lit/Test.py
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/lit/cl_arguments.py
  llvm/utils/lit/lit/discovery.py
  llvm/utils/lit/lit/formats/base.py
  llvm/utils/lit/lit/formats/googletest.py
  llvm/utils/lit/lit/main.py
  llvm/utils/lit/tests/Inputs/early-tests/aaa.txt
  llvm/utils/lit/tests/Inputs/early-tests/bbb.txt
  llvm/utils/lit/tests/Inputs/early-tests/lit.cfg
  llvm/utils/lit/tests/Inputs/early-tests/subdir/ccc.txt
  llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
  llvm/utils/lit/tests/Inputs/reorder/aaa.txt
  llvm/utils/lit/tests/Inputs/reorder/bbb.txt
  llvm/utils/lit/tests/Inputs/reorder/lit.cfg
  llvm/utils/lit/tests/Inputs/reorder/subdir/ccc.txt
  llvm/utils/lit/tests/early-tests.py
  llvm/utils/lit/tests/ignore-fail.py
  llvm/utils/lit/tests/reorder.py
  llvm/utils/lit/tests/shtest-shell.py
  mlir/test/Unit/lit.cfg.py

Index: mlir/test/Unit/lit.cfg.py
===
--- mlir/test/Unit/lit.cfg.py
+++ mlir/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = []
 
-# is_early; Request to run this suite early.
-config.is_early = True
-
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.mlir_obj_root, 'unittests')
Index: llvm/utils/lit/tests/shtest-shell.py
===
--- llvm/utils/lit/tests/shtest-shell.py
+++ llvm/utils/lit/tests/shtest-shell.py
@@ -8,6 +8,8 @@
 #
 # Test again in non-UTF shell to catch potential errors with python 2 seen
 # on stdout-encoding.txt
+# FIXME: lit's testing sets source_root == exec_root which complicates running lit more than once per test
+# RUN: rm -f %{inputs}/shtest-shell/.lit_test_times.txt
 # RUN: env PYTHONIOENCODING=ascii not %{lit} -j 1 -a %{inputs}/shtest-shell > %t.ascii.out
 # FIXME: Temporarily dump test output so we can debug failing tests on
 # buildbots.
Index: llvm/utils/lit/tests/reorder.py
===
--- /dev/null
+++ llvm/utils/lit/tests/reorder.py
@@ -0,0 +1,11 @@
+## Check that we can reorder test starts
+
+# RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# RUN: %{lit} -j1 %{inputs}/reorder | FileCheck %s
+# RUN: not diff %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+
+# CHECK: -- Testing: 3 tests, 1 workers --
+# CHECK-NEXT: PASS: reorder :: subdir/ccc.txt
+# CHECK-NEXT: PASS: reorder :: bbb.txt
+# CHECK-NEXT: PASS: reorder :: aaa.txt
+# CHECK: Passed: 3
Index: llvm/utils/lit/tests/ignore-fail.py
===
--- llvm/utils/lit/tests/ignore-fail.py
+++ llvm/utils/lit/tests/ignore-fail.py
@@ -6,10 +6,10 @@
 
 # END.
 
-# CHECK: FAIL: ignore-fail :: fail.txt
-# CHECK: UNRESOLVED: ignore-fail :: unresolved.txt
-# CHECK: XFAIL: ignore-fail :: xfail.txt
-# CHECK: XPASS: ignore-fail :: xpass.txt
+# CHECK-DAG: FAIL: ignore-fail :: fail.txt
+# CHECK-DAG: UNRESOLVED: ignore-fail :: unresolved.txt
+# CHECK-DAG: XFAIL: ignore-fail :: xfail.txt
+# CHECK-DAG: XPASS: ignore-fail :: xpass.txt
 
 #  CHECK: Testing Time:
 # CHECK-NEXT:   Expectedly Failed : 1
Index: llvm/utils/lit/tests/early-tests.py
===
--- 

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki updated this revision to Diff 329659.
davezarzycki added a comment.

I renamed the `file` variable as requested.

I also found a workaround to the performance problem I referred to earlier. Now 
no new parameter is required during test discovery and therefore the diff is 
both simpler and more effective because now programmatic test creators get test 
record-and-replay support for free.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

Files:
  llvm/docs/CommandGuide/lit.rst
  llvm/test/Unit/lit.cfg.py
  llvm/utils/lit/lit/Test.py
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/lit/cl_arguments.py
  llvm/utils/lit/lit/discovery.py
  llvm/utils/lit/lit/main.py
  llvm/utils/lit/tests/Inputs/early-tests/aaa.txt
  llvm/utils/lit/tests/Inputs/early-tests/bbb.txt
  llvm/utils/lit/tests/Inputs/early-tests/lit.cfg
  llvm/utils/lit/tests/Inputs/early-tests/subdir/ccc.txt
  llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
  llvm/utils/lit/tests/Inputs/reorder/aaa.txt
  llvm/utils/lit/tests/Inputs/reorder/bbb.txt
  llvm/utils/lit/tests/Inputs/reorder/lit.cfg
  llvm/utils/lit/tests/Inputs/reorder/subdir/ccc.txt
  llvm/utils/lit/tests/early-tests.py
  llvm/utils/lit/tests/ignore-fail.py
  llvm/utils/lit/tests/reorder.py
  llvm/utils/lit/tests/shtest-shell.py
  mlir/test/Unit/lit.cfg.py

Index: mlir/test/Unit/lit.cfg.py
===
--- mlir/test/Unit/lit.cfg.py
+++ mlir/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = []
 
-# is_early; Request to run this suite early.
-config.is_early = True
-
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.mlir_obj_root, 'unittests')
Index: llvm/utils/lit/tests/shtest-shell.py
===
--- llvm/utils/lit/tests/shtest-shell.py
+++ llvm/utils/lit/tests/shtest-shell.py
@@ -8,6 +8,8 @@
 #
 # Test again in non-UTF shell to catch potential errors with python 2 seen
 # on stdout-encoding.txt
+# FIXME: lit's testing sets source_root == exec_root which complicates running lit more than once per test
+# RUN: rm -f %{inputs}/shtest-shell/.lit_test_times.txt
 # RUN: env PYTHONIOENCODING=ascii not %{lit} -j 1 -a %{inputs}/shtest-shell > %t.ascii.out
 # FIXME: Temporarily dump test output so we can debug failing tests on
 # buildbots.
Index: llvm/utils/lit/tests/reorder.py
===
--- /dev/null
+++ llvm/utils/lit/tests/reorder.py
@@ -0,0 +1,11 @@
+## Check that we can reorder test starts
+
+# RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# RUN: %{lit} -j1 %{inputs}/reorder | FileCheck %s
+# RUN: not diff %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+
+# CHECK: -- Testing: 3 tests, 1 workers --
+# CHECK-NEXT: PASS: reorder :: subdir/ccc.txt
+# CHECK-NEXT: PASS: reorder :: bbb.txt
+# CHECK-NEXT: PASS: reorder :: aaa.txt
+# CHECK: Passed: 3
Index: llvm/utils/lit/tests/ignore-fail.py
===
--- llvm/utils/lit/tests/ignore-fail.py
+++ llvm/utils/lit/tests/ignore-fail.py
@@ -6,10 +6,10 @@
 
 # END.
 
-# CHECK: FAIL: ignore-fail :: fail.txt
-# CHECK: UNRESOLVED: ignore-fail :: unresolved.txt
-# CHECK: XFAIL: ignore-fail :: xfail.txt
-# CHECK: XPASS: ignore-fail :: xpass.txt
+# CHECK-DAG: FAIL: ignore-fail :: fail.txt
+# CHECK-DAG: UNRESOLVED: ignore-fail :: unresolved.txt
+# CHECK-DAG: XFAIL: ignore-fail :: xfail.txt
+# CHECK-DAG: XPASS: ignore-fail :: xpass.txt
 
 #  CHECK: Testing Time:
 # CHECK-NEXT:   Expectedly Failed : 1
Index: llvm/utils/lit/tests/early-tests.py
===
--- llvm/utils/lit/tests/early-tests.py
+++ /dev/null
@@ -1,9 +0,0 @@
-## Check that we can run tests early.
-
-# RUN: %{lit} -j1 %{inputs}/early-tests | FileCheck %s
-
-# CHECK: -- Testing: 3 tests, 1 workers --
-# CHECK-NEXT: PASS: early-tests :: subdir/ccc.txt
-# CHECK-NEXT: PASS: early-tests :: aaa.txt
-# CHECK-NEXT: PASS: early-tests :: bbb.txt
-# CHECK: Passed: 3
Index: llvm/utils/lit/tests/Inputs/reorder/lit.cfg
===
--- llvm/utils/lit/tests/Inputs/reorder/lit.cfg
+++ llvm/utils/lit/tests/Inputs/reorder/lit.cfg
@@ -1,7 +1,6 @@
 import lit.formats
-config.name = 'early-tests'
+config.name = 'reorder'
 config.suffixes = ['.txt']
 config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
-config.early_tests = { "subdir/ccc.txt" }
Index: llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
===

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki removed 1 blocking reviewer(s): libc++.
davezarzycki added a comment.

I'm downgrading libcxx as a reviewer to non-blocking because the latest patch 
no longer requires changes to their project.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki updated this revision to Diff 329277.
davezarzycki added a comment.
Herald added subscribers: libcxx-commits, lldb-commits, arichardson.
Herald added projects: LLDB, libc++.
Herald added a reviewer: libc++.

I believe I've addressed all of the feedback to date.

I've also fixed the regression. I had hoped to avoid adding `test_times` as a 
parameter to a bunch of methods, but adding that dictionary to the test suite 
makes `lit` effectively single threaded as the worker threads fought over what 
I assume is the global python interpreter lock.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

Files:
  libcxx/utils/libcxx/test/format.py
  libcxx/utils/libcxx/test/googlebenchmark.py
  lldb/test/API/lldbtest.py
  llvm/docs/CommandGuide/lit.rst
  llvm/test/Unit/lit.cfg.py
  llvm/utils/lit/lit/Test.py
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/lit/cl_arguments.py
  llvm/utils/lit/lit/discovery.py
  llvm/utils/lit/lit/formats/base.py
  llvm/utils/lit/lit/formats/googletest.py
  llvm/utils/lit/lit/main.py
  llvm/utils/lit/tests/Inputs/early-tests/aaa.txt
  llvm/utils/lit/tests/Inputs/early-tests/bbb.txt
  llvm/utils/lit/tests/Inputs/early-tests/lit.cfg
  llvm/utils/lit/tests/Inputs/early-tests/subdir/ccc.txt
  llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
  llvm/utils/lit/tests/Inputs/reorder/aaa.txt
  llvm/utils/lit/tests/Inputs/reorder/bbb.txt
  llvm/utils/lit/tests/Inputs/reorder/lit.cfg
  llvm/utils/lit/tests/Inputs/reorder/subdir/ccc.txt
  llvm/utils/lit/tests/early-tests.py
  llvm/utils/lit/tests/ignore-fail.py
  llvm/utils/lit/tests/reorder.py
  llvm/utils/lit/tests/shtest-shell.py
  mlir/test/Unit/lit.cfg.py

Index: mlir/test/Unit/lit.cfg.py
===
--- mlir/test/Unit/lit.cfg.py
+++ mlir/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = []
 
-# is_early; Request to run this suite early.
-config.is_early = True
-
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.mlir_obj_root, 'unittests')
Index: llvm/utils/lit/tests/shtest-shell.py
===
--- llvm/utils/lit/tests/shtest-shell.py
+++ llvm/utils/lit/tests/shtest-shell.py
@@ -8,6 +8,8 @@
 #
 # Test again in non-UTF shell to catch potential errors with python 2 seen
 # on stdout-encoding.txt
+# FIXME: lit's testing sets source_root == exec_root which complicates running lit more than once per test
+# RUN: rm -f %{inputs}/shtest-shell/.lit_test_times.txt
 # RUN: env PYTHONIOENCODING=ascii not %{lit} -j 1 -a %{inputs}/shtest-shell > %t.ascii.out
 # FIXME: Temporarily dump test output so we can debug failing tests on
 # buildbots.
Index: llvm/utils/lit/tests/reorder.py
===
--- /dev/null
+++ llvm/utils/lit/tests/reorder.py
@@ -0,0 +1,11 @@
+## Check that we can reorder test starts
+
+# RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# RUN: %{lit} -j1 %{inputs}/reorder | FileCheck %s
+# RUN: not diff %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+
+# CHECK: -- Testing: 3 tests, 1 workers --
+# CHECK-NEXT: PASS: reorder :: subdir/ccc.txt
+# CHECK-NEXT: PASS: reorder :: bbb.txt
+# CHECK-NEXT: PASS: reorder :: aaa.txt
+# CHECK: Passed: 3
Index: llvm/utils/lit/tests/ignore-fail.py
===
--- llvm/utils/lit/tests/ignore-fail.py
+++ llvm/utils/lit/tests/ignore-fail.py
@@ -6,10 +6,10 @@
 
 # END.
 
-# CHECK: FAIL: ignore-fail :: fail.txt
-# CHECK: UNRESOLVED: ignore-fail :: unresolved.txt
-# CHECK: XFAIL: ignore-fail :: xfail.txt
-# CHECK: XPASS: ignore-fail :: xpass.txt
+# CHECK-DAG: FAIL: ignore-fail :: fail.txt
+# CHECK-DAG: UNRESOLVED: ignore-fail :: unresolved.txt
+# CHECK-DAG: XFAIL: ignore-fail :: xfail.txt
+# CHECK-DAG: XPASS: ignore-fail :: xpass.txt
 
 #  CHECK: Testing Time:
 # CHECK-NEXT:   Expectedly Failed : 1
Index: llvm/utils/lit/tests/early-tests.py
===
--- llvm/utils/lit/tests/early-tests.py
+++ /dev/null
@@ -1,9 +0,0 @@
-## Check that we can run tests early.
-
-# RUN: %{lit} -j1 %{inputs}/early-tests | FileCheck %s
-
-# CHECK: -- Testing: 3 tests, 1 workers --
-# CHECK-NEXT: PASS: early-tests :: subdir/ccc.txt
-# CHECK-NEXT: PASS: early-tests :: aaa.txt
-# CHECK-NEXT: PASS: early-tests :: bbb.txt
-# CHECK: Passed: 3
Index: llvm/utils/lit/tests/Inputs/reorder/lit.cfg
===
--- llvm/utils/lit/tests/Inputs/reorder/lit.cfg
+++ llvm/utils/lit/tests/Inputs/reorder/lit.cfg
@@ -1,7 +1,6 @@
 impor

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki updated this revision to Diff 330194.
davezarzycki added a comment.

I've made all of the requested changes to date. Two notes:

1. One cannot simply omit the first argument to `split()` if the second is 
provided. As I just learned, python will complain. I switched the first 
parameter to `None` as a compromise.
2. For the record, I disagree with the use of the word "run" over "start" but 
this isn't something that I care enough about to resist changing the patch. 
Strictly speaking, `lit` only controls when tests start, not anything about how 
they run. With enough cores, tests will complete in a fairly predictable order 
despite any effort to start tests in a different order (or randomly).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

Files:
  llvm/docs/CommandGuide/lit.rst
  llvm/test/Unit/lit.cfg.py
  llvm/utils/lit/lit/Test.py
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/lit/cl_arguments.py
  llvm/utils/lit/lit/discovery.py
  llvm/utils/lit/lit/main.py
  llvm/utils/lit/tests/Inputs/early-tests/aaa.txt
  llvm/utils/lit/tests/Inputs/early-tests/bbb.txt
  llvm/utils/lit/tests/Inputs/early-tests/lit.cfg
  llvm/utils/lit/tests/Inputs/early-tests/subdir/ccc.txt
  llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
  llvm/utils/lit/tests/Inputs/reorder/aaa.txt
  llvm/utils/lit/tests/Inputs/reorder/bbb.txt
  llvm/utils/lit/tests/Inputs/reorder/lit.cfg
  llvm/utils/lit/tests/Inputs/reorder/subdir/ccc.txt
  llvm/utils/lit/tests/early-tests.py
  llvm/utils/lit/tests/ignore-fail.py
  llvm/utils/lit/tests/reorder.py
  llvm/utils/lit/tests/shtest-shell.py
  mlir/test/Unit/lit.cfg.py

Index: mlir/test/Unit/lit.cfg.py
===
--- mlir/test/Unit/lit.cfg.py
+++ mlir/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = []
 
-# is_early; Request to run this suite early.
-config.is_early = True
-
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.mlir_obj_root, 'unittests')
Index: llvm/utils/lit/tests/shtest-shell.py
===
--- llvm/utils/lit/tests/shtest-shell.py
+++ llvm/utils/lit/tests/shtest-shell.py
@@ -8,6 +8,8 @@
 #
 # Test again in non-UTF shell to catch potential errors with python 2 seen
 # on stdout-encoding.txt
+# FIXME: lit's testing sets source_root == exec_root which complicates running lit more than once per test
+# RUN: rm -f %{inputs}/shtest-shell/.lit_test_times.txt
 # RUN: env PYTHONIOENCODING=ascii not %{lit} -j 1 -a %{inputs}/shtest-shell > %t.ascii.out
 # FIXME: Temporarily dump test output so we can debug failing tests on
 # buildbots.
Index: llvm/utils/lit/tests/reorder.py
===
--- /dev/null
+++ llvm/utils/lit/tests/reorder.py
@@ -0,0 +1,12 @@
+## Check that we can reorder test runs.
+
+# RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# RUN: %{lit} -j1 %{inputs}/reorder | FileCheck %s
+# RUN: not diff %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# END.
+
+# CHECK: -- Testing: 3 tests, 1 workers --
+# CHECK-NEXT: PASS: reorder :: subdir/ccc.txt
+# CHECK-NEXT: PASS: reorder :: bbb.txt
+# CHECK-NEXT: PASS: reorder :: aaa.txt
+# CHECK: Passed: 3
Index: llvm/utils/lit/tests/ignore-fail.py
===
--- llvm/utils/lit/tests/ignore-fail.py
+++ llvm/utils/lit/tests/ignore-fail.py
@@ -6,10 +6,10 @@
 
 # END.
 
-# CHECK: FAIL: ignore-fail :: fail.txt
-# CHECK: UNRESOLVED: ignore-fail :: unresolved.txt
-# CHECK: XFAIL: ignore-fail :: xfail.txt
-# CHECK: XPASS: ignore-fail :: xpass.txt
+# CHECK-DAG: FAIL: ignore-fail :: fail.txt
+# CHECK-DAG: UNRESOLVED: ignore-fail :: unresolved.txt
+# CHECK-DAG: XFAIL: ignore-fail :: xfail.txt
+# CHECK-DAG: XPASS: ignore-fail :: xpass.txt
 
 #  CHECK: Testing Time:
 # CHECK-NEXT:   Expectedly Failed : 1
Index: llvm/utils/lit/tests/early-tests.py
===
--- llvm/utils/lit/tests/early-tests.py
+++ /dev/null
@@ -1,9 +0,0 @@
-## Check that we can run tests early.
-
-# RUN: %{lit} -j1 %{inputs}/early-tests | FileCheck %s
-
-# CHECK: -- Testing: 3 tests, 1 workers --
-# CHECK-NEXT: PASS: early-tests :: subdir/ccc.txt
-# CHECK-NEXT: PASS: early-tests :: aaa.txt
-# CHECK-NEXT: PASS: early-tests :: bbb.txt
-# CHECK: Passed: 3
Index: llvm/utils/lit/tests/Inputs/reorder/lit.cfg
===
--- llvm/utils/lit/tests/Inputs/reorder/lit.cfg
+++ llvm/utils/lit/tests/Inputs/reorder/lit.cfg
@@ -1,7 +1,6 @@
 import lit.formats
-config.name = 'e

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki updated this revision to Diff 330200.
davezarzycki added a comment.

I fixed the call to split as requested. Being unfamiliar with python, I didn't 
know that it supported keyword arguments / parameter labels. Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

Files:
  llvm/docs/CommandGuide/lit.rst
  llvm/test/Unit/lit.cfg.py
  llvm/utils/lit/lit/Test.py
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/lit/cl_arguments.py
  llvm/utils/lit/lit/discovery.py
  llvm/utils/lit/lit/main.py
  llvm/utils/lit/tests/Inputs/early-tests/aaa.txt
  llvm/utils/lit/tests/Inputs/early-tests/bbb.txt
  llvm/utils/lit/tests/Inputs/early-tests/lit.cfg
  llvm/utils/lit/tests/Inputs/early-tests/subdir/ccc.txt
  llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
  llvm/utils/lit/tests/Inputs/reorder/aaa.txt
  llvm/utils/lit/tests/Inputs/reorder/bbb.txt
  llvm/utils/lit/tests/Inputs/reorder/lit.cfg
  llvm/utils/lit/tests/Inputs/reorder/subdir/ccc.txt
  llvm/utils/lit/tests/early-tests.py
  llvm/utils/lit/tests/ignore-fail.py
  llvm/utils/lit/tests/reorder.py
  llvm/utils/lit/tests/shtest-shell.py
  mlir/test/Unit/lit.cfg.py

Index: mlir/test/Unit/lit.cfg.py
===
--- mlir/test/Unit/lit.cfg.py
+++ mlir/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = []
 
-# is_early; Request to run this suite early.
-config.is_early = True
-
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.mlir_obj_root, 'unittests')
Index: llvm/utils/lit/tests/shtest-shell.py
===
--- llvm/utils/lit/tests/shtest-shell.py
+++ llvm/utils/lit/tests/shtest-shell.py
@@ -8,6 +8,8 @@
 #
 # Test again in non-UTF shell to catch potential errors with python 2 seen
 # on stdout-encoding.txt
+# FIXME: lit's testing sets source_root == exec_root which complicates running lit more than once per test
+# RUN: rm -f %{inputs}/shtest-shell/.lit_test_times.txt
 # RUN: env PYTHONIOENCODING=ascii not %{lit} -j 1 -a %{inputs}/shtest-shell > %t.ascii.out
 # FIXME: Temporarily dump test output so we can debug failing tests on
 # buildbots.
Index: llvm/utils/lit/tests/reorder.py
===
--- /dev/null
+++ llvm/utils/lit/tests/reorder.py
@@ -0,0 +1,12 @@
+## Check that we can reorder test runs.
+
+# RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# RUN: %{lit} -j1 %{inputs}/reorder | FileCheck %s
+# RUN: not diff %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# END.
+
+# CHECK: -- Testing: 3 tests, 1 workers --
+# CHECK-NEXT: PASS: reorder :: subdir/ccc.txt
+# CHECK-NEXT: PASS: reorder :: bbb.txt
+# CHECK-NEXT: PASS: reorder :: aaa.txt
+# CHECK: Passed: 3
Index: llvm/utils/lit/tests/ignore-fail.py
===
--- llvm/utils/lit/tests/ignore-fail.py
+++ llvm/utils/lit/tests/ignore-fail.py
@@ -6,10 +6,10 @@
 
 # END.
 
-# CHECK: FAIL: ignore-fail :: fail.txt
-# CHECK: UNRESOLVED: ignore-fail :: unresolved.txt
-# CHECK: XFAIL: ignore-fail :: xfail.txt
-# CHECK: XPASS: ignore-fail :: xpass.txt
+# CHECK-DAG: FAIL: ignore-fail :: fail.txt
+# CHECK-DAG: UNRESOLVED: ignore-fail :: unresolved.txt
+# CHECK-DAG: XFAIL: ignore-fail :: xfail.txt
+# CHECK-DAG: XPASS: ignore-fail :: xpass.txt
 
 #  CHECK: Testing Time:
 # CHECK-NEXT:   Expectedly Failed : 1
Index: llvm/utils/lit/tests/early-tests.py
===
--- llvm/utils/lit/tests/early-tests.py
+++ /dev/null
@@ -1,9 +0,0 @@
-## Check that we can run tests early.
-
-# RUN: %{lit} -j1 %{inputs}/early-tests | FileCheck %s
-
-# CHECK: -- Testing: 3 tests, 1 workers --
-# CHECK-NEXT: PASS: early-tests :: subdir/ccc.txt
-# CHECK-NEXT: PASS: early-tests :: aaa.txt
-# CHECK-NEXT: PASS: early-tests :: bbb.txt
-# CHECK: Passed: 3
Index: llvm/utils/lit/tests/Inputs/reorder/lit.cfg
===
--- llvm/utils/lit/tests/Inputs/reorder/lit.cfg
+++ llvm/utils/lit/tests/Inputs/reorder/lit.cfg
@@ -1,7 +1,6 @@
 import lit.formats
-config.name = 'early-tests'
+config.name = 'reorder'
 config.suffixes = ['.txt']
 config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
-config.early_tests = { "subdir/ccc.txt" }
Index: llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
===
--- /dev/null
+++ llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
@@ -0,0 +1,3 @@
+3.0 subdir/ccc.txt
+2.0 bbb.txt
+0.1 aaa.txt
Inde

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Might somebody be willing to sign off on this change (this week or next)? I'd 
like to cherry-pick it to Swift's LLVM branch. Thanks for all the feedback so 
far.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki updated this revision to Diff 330907.
davezarzycki added a comment.

Removed the commented only-failures and fixed a nit before committing.

Thanks everybody for the various rounds of feedback. I'm quite happy with how 
this turned out.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

Files:
  llvm/docs/CommandGuide/lit.rst
  llvm/test/Unit/lit.cfg.py
  llvm/utils/lit/lit/Test.py
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/lit/cl_arguments.py
  llvm/utils/lit/lit/discovery.py
  llvm/utils/lit/lit/main.py
  llvm/utils/lit/tests/Inputs/early-tests/aaa.txt
  llvm/utils/lit/tests/Inputs/early-tests/bbb.txt
  llvm/utils/lit/tests/Inputs/early-tests/lit.cfg
  llvm/utils/lit/tests/Inputs/early-tests/subdir/ccc.txt
  llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
  llvm/utils/lit/tests/Inputs/reorder/aaa.txt
  llvm/utils/lit/tests/Inputs/reorder/bbb.txt
  llvm/utils/lit/tests/Inputs/reorder/lit.cfg
  llvm/utils/lit/tests/Inputs/reorder/subdir/ccc.txt
  llvm/utils/lit/tests/early-tests.py
  llvm/utils/lit/tests/ignore-fail.py
  llvm/utils/lit/tests/reorder.py
  llvm/utils/lit/tests/shtest-shell.py
  mlir/test/Unit/lit.cfg.py

Index: mlir/test/Unit/lit.cfg.py
===
--- mlir/test/Unit/lit.cfg.py
+++ mlir/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = []
 
-# is_early; Request to run this suite early.
-config.is_early = True
-
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.mlir_obj_root, 'unittests')
Index: llvm/utils/lit/tests/shtest-shell.py
===
--- llvm/utils/lit/tests/shtest-shell.py
+++ llvm/utils/lit/tests/shtest-shell.py
@@ -8,6 +8,8 @@
 #
 # Test again in non-UTF shell to catch potential errors with python 2 seen
 # on stdout-encoding.txt
+# FIXME: lit's testing sets source_root == exec_root which complicates running lit more than once per test.
+# RUN: rm -f %{inputs}/shtest-shell/.lit_test_times.txt
 # RUN: env PYTHONIOENCODING=ascii not %{lit} -j 1 -a %{inputs}/shtest-shell > %t.ascii.out
 # FIXME: Temporarily dump test output so we can debug failing tests on
 # buildbots.
Index: llvm/utils/lit/tests/reorder.py
===
--- /dev/null
+++ llvm/utils/lit/tests/reorder.py
@@ -0,0 +1,12 @@
+## Check that we can reorder test runs.
+
+# RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# RUN: %{lit} -j1 %{inputs}/reorder | FileCheck %s
+# RUN: not diff %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# END.
+
+# CHECK: -- Testing: 3 tests, 1 workers --
+# CHECK-NEXT: PASS: reorder :: subdir/ccc.txt
+# CHECK-NEXT: PASS: reorder :: bbb.txt
+# CHECK-NEXT: PASS: reorder :: aaa.txt
+# CHECK: Passed: 3
Index: llvm/utils/lit/tests/ignore-fail.py
===
--- llvm/utils/lit/tests/ignore-fail.py
+++ llvm/utils/lit/tests/ignore-fail.py
@@ -6,10 +6,10 @@
 
 # END.
 
-# CHECK: FAIL: ignore-fail :: fail.txt
-# CHECK: UNRESOLVED: ignore-fail :: unresolved.txt
-# CHECK: XFAIL: ignore-fail :: xfail.txt
-# CHECK: XPASS: ignore-fail :: xpass.txt
+# CHECK-DAG: FAIL: ignore-fail :: fail.txt
+# CHECK-DAG: UNRESOLVED: ignore-fail :: unresolved.txt
+# CHECK-DAG: XFAIL: ignore-fail :: xfail.txt
+# CHECK-DAG: XPASS: ignore-fail :: xpass.txt
 
 #  CHECK: Testing Time:
 # CHECK-NEXT:   Expectedly Failed : 1
Index: llvm/utils/lit/tests/early-tests.py
===
--- llvm/utils/lit/tests/early-tests.py
+++ /dev/null
@@ -1,9 +0,0 @@
-## Check that we can run tests early.
-
-# RUN: %{lit} -j1 %{inputs}/early-tests | FileCheck %s
-
-# CHECK: -- Testing: 3 tests, 1 workers --
-# CHECK-NEXT: PASS: early-tests :: subdir/ccc.txt
-# CHECK-NEXT: PASS: early-tests :: aaa.txt
-# CHECK-NEXT: PASS: early-tests :: bbb.txt
-# CHECK: Passed: 3
Index: llvm/utils/lit/tests/Inputs/reorder/lit.cfg
===
--- llvm/utils/lit/tests/Inputs/reorder/lit.cfg
+++ llvm/utils/lit/tests/Inputs/reorder/lit.cfg
@@ -1,7 +1,6 @@
 import lit.formats
-config.name = 'early-tests'
+config.name = 'reorder'
 config.suffixes = ['.txt']
 config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
-config.early_tests = { "subdir/ccc.txt" }
Index: llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
===
--- /dev/null
+++ llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
@@ -0,0 +1,3 @@
+3.0 subdir/ccc.txt
+2.0 bbb.t

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

That implies a different testing environment. How different is the Windows 
testing environment compared to Unix? Do you specifically have a custom testing 
setup?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1d297f90649d: [lit] Sort test start times based on prior 
test timing data (authored by davezarzycki).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

Files:
  llvm/docs/CommandGuide/lit.rst
  llvm/test/Unit/lit.cfg.py
  llvm/utils/lit/lit/Test.py
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/lit/cl_arguments.py
  llvm/utils/lit/lit/discovery.py
  llvm/utils/lit/lit/main.py
  llvm/utils/lit/tests/Inputs/early-tests/aaa.txt
  llvm/utils/lit/tests/Inputs/early-tests/bbb.txt
  llvm/utils/lit/tests/Inputs/early-tests/lit.cfg
  llvm/utils/lit/tests/Inputs/early-tests/subdir/ccc.txt
  llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
  llvm/utils/lit/tests/Inputs/reorder/aaa.txt
  llvm/utils/lit/tests/Inputs/reorder/bbb.txt
  llvm/utils/lit/tests/Inputs/reorder/lit.cfg
  llvm/utils/lit/tests/Inputs/reorder/subdir/ccc.txt
  llvm/utils/lit/tests/early-tests.py
  llvm/utils/lit/tests/ignore-fail.py
  llvm/utils/lit/tests/reorder.py
  llvm/utils/lit/tests/shtest-shell.py
  mlir/test/Unit/lit.cfg.py

Index: mlir/test/Unit/lit.cfg.py
===
--- mlir/test/Unit/lit.cfg.py
+++ mlir/test/Unit/lit.cfg.py
@@ -13,9 +13,6 @@
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = []
 
-# is_early; Request to run this suite early.
-config.is_early = True
-
 # test_source_root: The root path where tests are located.
 # test_exec_root: The root path where tests should be run.
 config.test_exec_root = os.path.join(config.mlir_obj_root, 'unittests')
Index: llvm/utils/lit/tests/shtest-shell.py
===
--- llvm/utils/lit/tests/shtest-shell.py
+++ llvm/utils/lit/tests/shtest-shell.py
@@ -8,6 +8,8 @@
 #
 # Test again in non-UTF shell to catch potential errors with python 2 seen
 # on stdout-encoding.txt
+# FIXME: lit's testing sets source_root == exec_root which complicates running lit more than once per test.
+# RUN: rm -f %{inputs}/shtest-shell/.lit_test_times.txt
 # RUN: env PYTHONIOENCODING=ascii not %{lit} -j 1 -a %{inputs}/shtest-shell > %t.ascii.out
 # FIXME: Temporarily dump test output so we can debug failing tests on
 # buildbots.
Index: llvm/utils/lit/tests/reorder.py
===
--- /dev/null
+++ llvm/utils/lit/tests/reorder.py
@@ -0,0 +1,12 @@
+## Check that we can reorder test runs.
+
+# RUN: cp %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# RUN: %{lit} -j1 %{inputs}/reorder | FileCheck %s
+# RUN: not diff %{inputs}/reorder/.lit_test_times.txt %{inputs}/reorder/.lit_test_times.txt.orig
+# END.
+
+# CHECK: -- Testing: 3 tests, 1 workers --
+# CHECK-NEXT: PASS: reorder :: subdir/ccc.txt
+# CHECK-NEXT: PASS: reorder :: bbb.txt
+# CHECK-NEXT: PASS: reorder :: aaa.txt
+# CHECK: Passed: 3
Index: llvm/utils/lit/tests/ignore-fail.py
===
--- llvm/utils/lit/tests/ignore-fail.py
+++ llvm/utils/lit/tests/ignore-fail.py
@@ -6,10 +6,10 @@
 
 # END.
 
-# CHECK: FAIL: ignore-fail :: fail.txt
-# CHECK: UNRESOLVED: ignore-fail :: unresolved.txt
-# CHECK: XFAIL: ignore-fail :: xfail.txt
-# CHECK: XPASS: ignore-fail :: xpass.txt
+# CHECK-DAG: FAIL: ignore-fail :: fail.txt
+# CHECK-DAG: UNRESOLVED: ignore-fail :: unresolved.txt
+# CHECK-DAG: XFAIL: ignore-fail :: xfail.txt
+# CHECK-DAG: XPASS: ignore-fail :: xpass.txt
 
 #  CHECK: Testing Time:
 # CHECK-NEXT:   Expectedly Failed : 1
Index: llvm/utils/lit/tests/early-tests.py
===
--- llvm/utils/lit/tests/early-tests.py
+++ /dev/null
@@ -1,9 +0,0 @@
-## Check that we can run tests early.
-
-# RUN: %{lit} -j1 %{inputs}/early-tests | FileCheck %s
-
-# CHECK: -- Testing: 3 tests, 1 workers --
-# CHECK-NEXT: PASS: early-tests :: subdir/ccc.txt
-# CHECK-NEXT: PASS: early-tests :: aaa.txt
-# CHECK-NEXT: PASS: early-tests :: bbb.txt
-# CHECK: Passed: 3
Index: llvm/utils/lit/tests/Inputs/reorder/lit.cfg
===
--- llvm/utils/lit/tests/Inputs/reorder/lit.cfg
+++ llvm/utils/lit/tests/Inputs/reorder/lit.cfg
@@ -1,7 +1,6 @@
 import lit.formats
-config.name = 'early-tests'
+config.name = 'reorder'
 config.suffixes = ['.txt']
 config.test_format = lit.formats.ShTest()
 config.test_source_root = None
 config.test_exec_root = None
-config.early_tests = { "subdir/ccc.txt" }
Index: llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
===
--- /dev/null
+++ llvm/utils/lit/tests/Inputs/reorder/.lit_test_times.txt
@@ -0,0 +1,3 @@
+3.0 subdir/ccc.txt
+2.0 bbb.

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Said differently, the Unix side seems to always run the lit tests from a 
fresh/clean setup, even for incremental builds. If Windows tests lit 
differently then ya, things will blow up.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Try: 49d0e115d5df40aa89339f4ace7a8dee378c03bb 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

Ah yes, of course. I've marked that test as unavailable on Window for now: 
61ca706461c5e1edc18526c9ddc3250fe074ed94 


In the long run, We'll need to figure out a way for people to commit 
cross-platform trace files but for now the more important record-and-reorder 
feature works cross platform.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

I've run out of time for today, but for the record, I think we should use slash 
as the canonical separator for the timing data file. The actual separator 
doesn't matter because these strings are never used as paths, just as keys into 
a dictionary. (Therefore any character would work.) This would let us harmonize 
the Unix and Windows timing data files.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

In D98179#2638882 , @mehdi_amini wrote:

>> No, I'm running lit and dumping into a file. sorting is not as easy as 
>> piping through sort, as I'm running with -vv (required).
>
> Since tests run in parallel, don't you already have some non-determinism in 
> the -vv output? Isn't it printing as test finish? (just curious, not pushing 
> back on anything here).

Right. Unless `-j1` is passed to lit, then the `lit` output is, strictly 
speaking, non-deterministic. That being said (and especially on a quiet 
machine), I do understand why people might feel that the output is consistent 
enough from run to run to be quasi-deterministic.

>> Anyway, sorting the list on lit side should be 1 line for some that knows 
>> where that line goes :)
>
> Yeah I agree that sorting on the lit side for the summary should be fairly 
> easy!

As far as I can tell, the `lit` architecture likes to "pay-as-you-go" with test 
output, so there isn't a good place to sort the output and I'd wager that a lot 
of people would be upset if the test output stalled until completion for 
sorting reasons.

If people feel that deleting the test timing data is onerous (and IMHO, I don't 
think that's a big ask), then I think the simplest solution is to add a 
`--ignore-timing-data` option to lit that ignores test timing data and gives 
people the old behavior (for whatever reason).

But I don't think the *default* lit behavior should be to ignore test failures 
and ignore test timing data. Both of these are about making the out-of-the-box 
experience for people more productive. If people have reasons to run tests in 
specific orders, then let's talk about those needs and why a lexical was 
convenient.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

In D98179#2639483 , @lebedev.ri wrote:

> In D98179#2639476 , @nlopes wrote:
>
>> I'm talking about sorting just the summary of failed tests, not the whole 
>> output. We need the whole -vv output, but that can be out of order.
>
> Aha! +1 then, i think sorting summary should be both easy and nice to have.

Agreed. Try: 5cbe2279f723f1cca1a542d95e7d9760e4f52240 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

In D98179#2639476 , @nlopes wrote:

> I'm talking about sorting just the summary of failed tests, not the whole 
> output. We need the whole -vv output, but that can be out of order.
>
> Why are timeouts important? Our use case is running Alive2 with the test 
> suite. Alive2 is heavy machinery and runs into timeouts. Running the tests in 
> roughly the same order every time is important to avoid timeout tests 
> flipping to failed or vice-versa. Plus slow tests = tests that consume a lot 
> of memory (in our scenario), so we can't bundle slow tests together.
> Adding a `--ignore-timing-data` would be great, yes! But I still feel that 
> sorting the list of failed tests is important for user experience. I diff 
> these all the time.

That still sounds incredibly brittle. If there is any variety in test machine 
performance, then any and all attempts at sorting should be futile because the 
underlying hardware will perturb different timeouts. Is this not your 
experience? How do you reconcile hardware performance and configuration details 
(like SMT) with timeout settings?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

In D98179#2638837 , @nlopes wrote:

> In D98179#2638801 , @mehdi_amini 
> wrote:
>
>>> Can we revert to the previous behavior please? The current behavior is not 
>>> user friendly. Thanks!
>>
>> To clarify: you care about the order in the final summary, not the actual 
>> execution order, right? (the goal of this patch is the latter, if it changes 
>> the former this is just a side-effect I believe)
>
> I don't love that the default test order was changed. For our use case that 
> isn't good, but I can just delete the `.lit_test_times.txt` file and get the 
> old fixed order. (the reason why having a fixed order is good is because of 
> timeouts. Also, slow tests often consume more memory in our setting, so it's 
> not good to run all the slow tests at the same time).

Can you elaborate more about your usage of timeouts? As far as I know, the 
feature exists only to detect wildly broken tests. If reordering the tests 
causes timeouts to fire, then either the timeout is too aggressive or the tests 
themselves were on the edge of failure anyway and any "unrelated" change could 
cause them to fail.

As to the resource management challenges, `lit` has an undocumented 
"parallelism group" feature that would probably help you keep resource usage in 
check and would be more far appropriate than lexical execution order.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D98179: [lit] Sort test start times based on prior test timing data

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

In D98179#2639554 , @nlopes wrote:

> In D98179#2639491 , @davezarzycki 
> wrote:
>
>> In D98179#2639476 , @nlopes wrote:
>>
>>> Why are timeouts important? Our use case is running Alive2 with the test 
>>> suite. Alive2 is heavy machinery and runs into timeouts. Running the tests 
>>> in roughly the same order every time is important to avoid timeout tests 
>>> flipping to failed or vice-versa. Plus slow tests = tests that consume a 
>>> lot of memory (in our scenario), so we can't bundle slow tests together.
>>> Adding a `--ignore-timing-data` would be great, yes! But I still feel that 
>>> sorting the list of failed tests is important for user experience. I diff 
>>> these all the time.
>>
>> That still sounds incredibly brittle. If there is any variety in test 
>> machine performance, then any and all attempts at sorting should be futile 
>> because the underlying hardware will perturb different timeouts. Is this not 
>> your experience? How do you reconcile hardware performance and configuration 
>> details (like SMT) with timeout settings?
>
> Of course it's brittle :)  Changing from a time-based setting to a 
> ticks-based system is ongoing work, such that resource exhaustion becomes 
> deterministic.
> Nevertheless, on a same machine, we don't see many test flips. It's quite 
> stable most of the times (just one test flip once in a while).

This seems really beyond the scope and purpose of sorting the tests.

If you don't mind and given that the workaround is trivial (delete the timing 
data), I'd like to hold off on adding `--ignore-timing-data`. If enough people 
complain then we can add that option. Is that okay with you?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98179/new/

https://reviews.llvm.org/D98179

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

An auto-bisecting cron job has identified this change as a regression on Fedora 
33 (x86-64). Can we get a quick fix or revert this? Here is the build output:

https://znu.io/dd391e1ef762d79f86112dc2480a89c9be066ce1-bisect.txt


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95713/new/

https://reviews.llvm.org/D95713

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

I've reverted this. If you need help debugging this, please let me know. Also, 
as a reminder, please see the attached build log from earlier in the 
conversation. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95713/new/

https://reviews.llvm.org/D95713

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

In D95713#2647548 , @teemperor wrote:

> The tests are failing because Dave's bot is running without enabled Python. 
> The same is true for the Windows bot. Putting the plugin behind `#ifdef 
> LLDB_ENABLE_PYTHON` should fix this.

My cron job is not intentionally disabling python support. That just happened 
to be the fallout from the normal feature detection dance before the build 
starts:

- Could NOT find SWIG (missing: SWIG_EXECUTABLE SWIG_DIR) (Required is at least 
version "3.0")
- SWIG 3 or later is required for Python support in LLDB but could not be found
- Could NOT find PythonAndSwig (missing: Python3_LIBRARIES Python3_INCLUDE_DIRS 
SWIG_EXECUTABLE)
- Enable Python scripting support in LLDB: FALSE


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95713/new/

https://reviews.llvm.org/D95713

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D95713: [lldb/Plugins] Add ScriptedProcess Process Plugin

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added inline comments.



Comment at: lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp:69
+
+  StructuredData::ObjectSP object_sp = GetInterface().CreatePluginObject(
+  m_launch_info.GetClassName().c_str(), target_sp,

mib wrote:
> stella.stamenova wrote:
> > This is where the AV happens on Windows. It looks like GetInterface returns 
> > null
> Thanks @stella.stamenova for taking a closer look at this. I don't know why 
> would the test call this code and crash, but I'll continue investigating it.
I've seen this crash on Fedora (x86-64) too so I don't think it's Windows 
specific.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D95713/new/

https://reviews.llvm.org/D95713

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

This is failing on my Fedora 34 (x86-64) box. Is this expected?

  FAIL: lldb-unit :: Host/./HostTests/TerminalTest.SaveRestoreRAII (78548 of 
79402)
   TEST 'lldb-unit :: 
Host/./HostTests/TerminalTest.SaveRestoreRAII' FAILED 
  Script:
  --
  /tmp/_update_lc/r/tools/lldb/unittests/Host/./HostTests 
--gtest_filter=TerminalTest.SaveRestoreRAII
  --
  Note: Google Test filter = TerminalTest.SaveRestoreRAII
  [==] Running 1 test from 1 test suite.
  [--] Global test environment set-up.
  [--] 1 test from TerminalTest
  [ RUN  ] TerminalTest.SaveRestoreRAII
  /home/dave/ro_s/lp/lldb/unittests/Host/posix/TerminalTest.cpp:93: Failure
  Expected equality of these values:
tcsetattr(m_pty.GetPrimaryFileDescriptor(), 0, &terminfo)
  Which is: -1
0
  [  FAILED  ] TerminalTest.SaveRestoreRAII (4 ms)
  [--] 1 test from TerminalTest (4 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test suite ran. (4 ms total)
  [  PASSED  ] 0 tests.
  [  FAILED  ] 1 test, listed below:
  [  FAILED  ] TerminalTest.SaveRestoreRAII
  
   1 FAILED TEST
  
  
  Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60.. 70.. 80.. 90..
  FAIL: lldb-unit :: Host/./HostTests/TerminalTest.SaveRestore (78561 of 79402)
   TEST 'lldb-unit :: 
Host/./HostTests/TerminalTest.SaveRestore' FAILED 
  Script:
  --
  /tmp/_update_lc/r/tools/lldb/unittests/Host/./HostTests 
--gtest_filter=TerminalTest.SaveRestore
  --
  Note: Google Test filter = TerminalTest.SaveRestore
  [==] Running 1 test from 1 test suite.
  [--] Global test environment set-up.
  [--] 1 test from TerminalTest
  [ RUN  ] TerminalTest.SaveRestore
  /home/dave/ro_s/lp/lldb/unittests/Host/posix/TerminalTest.cpp:121: Failure
  Expected equality of these values:
tcsetattr(m_pty.GetPrimaryFileDescriptor(), 0, &terminfo)
  Which is: -1
0
  [  FAILED  ] TerminalTest.SaveRestore (4 ms)
  [--] 1 test from TerminalTest (4 ms total)
  
  [--] Global test environment tear-down
  [==] 1 test from 1 test suite ran. (4 ms total)
  [  PASSED  ] 0 tests.
  [  FAILED  ] 1 test, listed below:
  [  FAILED  ] TerminalTest.SaveRestore
  
   1 FAILED TEST


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110962/new/

https://reviews.llvm.org/D110962

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

errno == 22 (EINVAL)

The FD in question is 3. Here is the output from `lsof`: HostTests 80108 dave   
 3u   CHR5,2  0t0   231 /dev/ptmx

Can we/I please revert this for now?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110962/new/

https://reviews.llvm.org/D110962

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

"the buildbot"? There are many. I'd be surprised if this weren't failing on at 
least one of them. It doesn't seem like subtle terminfo behavior is essential 
to this test. Can we please trim down the adjustments to only changing the 
speed? For example:

  diff --git i/lldb/unittests/Host/posix/TerminalTest.cpp 
w/lldb/unittests/Host/posix/TerminalTest.cpp
  index 
ecdb5480216439903b9fc12c39b3d47cb62f9134..e865d44bf6cb9085f07c11e06be34f33a7bd32b9
 100644
  --- i/lldb/unittests/Host/posix/TerminalTest.cpp
  +++ w/lldb/unittests/Host/posix/TerminalTest.cpp
  @@ -80,14 +80,8 @@ TEST_F(TerminalTest, SaveRestoreRAII) {
   terminfo = orig_terminfo;
  
   // make some arbitrary changes
  -terminfo.c_iflag ^= IGNPAR | INLCR;
  -terminfo.c_oflag ^= OPOST | OCRNL;
  -terminfo.c_cflag ^= PARENB | PARODD;
  -terminfo.c_lflag ^= ICANON | ECHO;
  -terminfo.c_cc[VEOF] ^= 8;
  -terminfo.c_cc[VEOL] ^= 4;
  -cfsetispeed(&terminfo, B9600);
  -cfsetospeed(&terminfo, B9600);
  +cfsetispeed(&terminfo, cfgetispeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
  +cfsetospeed(&terminfo, cfgetospeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
  
   ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, 
&terminfo),
 0);
  @@ -109,14 +103,8 @@ TEST_F(TerminalTest, SaveRestore) {
 terminfo = orig_terminfo;
  
 // make some arbitrary changes
  -  terminfo.c_iflag ^= IGNPAR | INLCR;
  -  terminfo.c_oflag ^= OPOST | OCRNL;
  -  terminfo.c_cflag ^= PARENB | PARODD;
  -  terminfo.c_lflag ^= ICANON | ECHO;
  -  terminfo.c_cc[VEOF] ^= 8;
  -  terminfo.c_cc[VEOL] ^= 4;
  -  cfsetispeed(&terminfo, B9600);
  -  cfsetospeed(&terminfo, B9600);
  +  cfsetispeed(&terminfo, cfgetispeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
  +  cfsetospeed(&terminfo, cfgetospeed(&orig_terminfo) == B9600 ? B4800 : 
B9600);
  
 ASSERT_EQ(tcsetattr(m_pty.GetPrimaryFileDescriptor(), TCSANOW, &terminfo), 
0);


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110962/new/

https://reviews.llvm.org/D110962

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

79bf032fe10546fd1d6e14c5ac8905f25c2b 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110962/new/

https://reviews.llvm.org/D110962

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

[Lldb-commits] [PATCH] D110962: [lldb] Add unit tests for Terminal API

[David Zarzycki via Phabricator via lldb-commits]

davezarzycki added a comment.

If you want to test more, please let me know and I can test them on my Fedora 
box.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110962/new/

https://reviews.llvm.org/D110962

___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits