[libc++] Optionally support filecheck-based tests #165769
Conversation
This patch adds support for filecheck tests in libc++'s test suite. However, it doesn't use LLVM's filecheck program, which requires building a bunch of LLVM utilities. Instead, it installs the Python port of filecheck at https://github.com/AntonLydike/filecheck which supports basically the same functionality. The test suite still works when filecheck is not available, since tests that use filecheck should be guarded on the `has-filecheck` Lit feature. This should make it possible to test several things that were previously impossible to test, especially for specific code generation.
llvmbot
added
libc++
|
@llvm/pr-subscribers-github-workflow Author: Louis Dionne (ldionne) ChangesThis patch adds support for filecheck tests in libc++'s test suite. However, it doesn't use LLVM's filecheck program, which requires building a bunch of LLVM utilities. Instead, it installs the Python port of filecheck at https://github.com/AntonLydike/filecheck which supports basically the same functionality. The test suite still works when filecheck is not available, since tests that use filecheck should be guarded on the This should make it possible to test several things that were previously impossible to test, especially for specific code generation. Full diff: https://github.com/llvm/llvm-project/pull/165769.diff 6 Files Affected:
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index 6c8f2cb45ee0a..6097174f5e553 100644
--- a/.github/workflows/libcxx-build-and-test.yaml
+++ b/.github/workflows/libcxx-build-and-test.yaml
@@ -221,7 +221,7 @@ jobs:
run: |
python3 -m venv .venv
source .venv/bin/activate
- python -m pip install psutil
+ pip install -r libcxx/test/requirements.txt
bash libcxx/utils/ci/run-buildbot ${{ matrix.config }}
- uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
if: always() # Upload artifacts even if the build or test suite fails
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index dbe69484abedf..d77c1c80fd307 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -23,6 +23,13 @@ Please see the `Lit Command Guide`_ for more information about LIT.
.. _LIT Command Guide: https://llvm.org/docs/CommandGuide/lit.html
+Dependencies
+------------
+
+The libc++ test suite has a few optional dependencies. These can be installed
+with ``pip install -r libcxx/test/requirements.txt``. Installing these dependencies
+will ensure that the maximum number of tests can be run.
+
Usage
-----
diff --git a/libcxx/test/requirements.txt b/libcxx/test/requirements.txt
new file mode 100644
index 0000000000000..842b8ca4ef901
--- /dev/null
+++ b/libcxx/test/requirements.txt
@@ -0,0 +1,5 @@
+#
+# This file defines Python requirements to run the libc++ test suite.
+#
+filecheck
+psutil
diff --git a/libcxx/test/selftest/filecheck.sh.cpp b/libcxx/test/selftest/filecheck.sh.cpp
new file mode 100644
index 0000000000000..911d22b414b3b
--- /dev/null
+++ b/libcxx/test/selftest/filecheck.sh.cpp
@@ -0,0 +1,15 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: has-filecheck
+
+// Make sure that we can use filecheck to write tests when the `has-filecheck`
+// Lit feature is defined.
+
+// RUN: echo "hello world" | filecheck %s
+// CHECK: hello world
diff --git a/libcxx/utils/ci/Dockerfile b/libcxx/utils/ci/Dockerfile
index d22deec4dadab..40592c3770c76 100644
--- a/libcxx/utils/ci/Dockerfile
+++ b/libcxx/utils/ci/Dockerfile
@@ -111,6 +111,9 @@ RUN sudo apt-get update \
xz-utils \
&& sudo rm -rf /var/lib/apt/lists/*
+COPY ../../test/requirements.txt .
+RUN python3 -m pip install -r requirements.txt
+
RUN <<EOF
set -e
wget -qO /tmp/ninja.gz https://github.com/ninja-build/ninja/releases/latest/download/ninja-linux.zip
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 7d6e78de343c5..21e86a44e745b 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -355,6 +355,14 @@ def _mingwSupportsModules(cfg):
name="has-no-zdump",
when=lambda cfg: runScriptExitCode(cfg, ["zdump --version"]) != 0,
),
+ # Whether the `filecheck` executable is available. Note that this corresponds to
+ # a Python port of LLVM's FileCheck, not LLVM's actual FileCheck program, since
+ # that one requires building parts of LLVM that we don't want to build when merely
+ # testing libc++.
+ Feature(
+ name="has-filecheck",
+ when=lambda cfg: runScriptExitCode(cfg, ["filecheck --version"]) == 0,
+ ),
]
# Deduce and add the test features that that are implied by the #defines in
|
|
@llvm/pr-subscribers-libcxx Author: Louis Dionne (ldionne) ChangesThis patch adds support for filecheck tests in libc++'s test suite. However, it doesn't use LLVM's filecheck program, which requires building a bunch of LLVM utilities. Instead, it installs the Python port of filecheck at https://github.com/AntonLydike/filecheck which supports basically the same functionality. The test suite still works when filecheck is not available, since tests that use filecheck should be guarded on the This should make it possible to test several things that were previously impossible to test, especially for specific code generation. Full diff: https://github.com/llvm/llvm-project/pull/165769.diff 6 Files Affected:
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index 6c8f2cb45ee0a..6097174f5e553 100644
--- a/.github/workflows/libcxx-build-and-test.yaml
+++ b/.github/workflows/libcxx-build-and-test.yaml
@@ -221,7 +221,7 @@ jobs:
run: |
python3 -m venv .venv
source .venv/bin/activate
- python -m pip install psutil
+ pip install -r libcxx/test/requirements.txt
bash libcxx/utils/ci/run-buildbot ${{ matrix.config }}
- uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
if: always() # Upload artifacts even if the build or test suite fails
diff --git a/libcxx/docs/TestingLibcxx.rst b/libcxx/docs/TestingLibcxx.rst
index dbe69484abedf..d77c1c80fd307 100644
--- a/libcxx/docs/TestingLibcxx.rst
+++ b/libcxx/docs/TestingLibcxx.rst
@@ -23,6 +23,13 @@ Please see the `Lit Command Guide`_ for more information about LIT.
.. _LIT Command Guide: https://llvm.org/docs/CommandGuide/lit.html
+Dependencies
+------------
+
+The libc++ test suite has a few optional dependencies. These can be installed
+with ``pip install -r libcxx/test/requirements.txt``. Installing these dependencies
+will ensure that the maximum number of tests can be run.
+
Usage
-----
diff --git a/libcxx/test/requirements.txt b/libcxx/test/requirements.txt
new file mode 100644
index 0000000000000..842b8ca4ef901
--- /dev/null
+++ b/libcxx/test/requirements.txt
@@ -0,0 +1,5 @@
+#
+# This file defines Python requirements to run the libc++ test suite.
+#
+filecheck
+psutil
diff --git a/libcxx/test/selftest/filecheck.sh.cpp b/libcxx/test/selftest/filecheck.sh.cpp
new file mode 100644
index 0000000000000..911d22b414b3b
--- /dev/null
+++ b/libcxx/test/selftest/filecheck.sh.cpp
@@ -0,0 +1,15 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+// REQUIRES: has-filecheck
+
+// Make sure that we can use filecheck to write tests when the `has-filecheck`
+// Lit feature is defined.
+
+// RUN: echo "hello world" | filecheck %s
+// CHECK: hello world
diff --git a/libcxx/utils/ci/Dockerfile b/libcxx/utils/ci/Dockerfile
index d22deec4dadab..40592c3770c76 100644
--- a/libcxx/utils/ci/Dockerfile
+++ b/libcxx/utils/ci/Dockerfile
@@ -111,6 +111,9 @@ RUN sudo apt-get update \
xz-utils \
&& sudo rm -rf /var/lib/apt/lists/*
+COPY ../../test/requirements.txt .
+RUN python3 -m pip install -r requirements.txt
+
RUN <<EOF
set -e
wget -qO /tmp/ninja.gz https://github.com/ninja-build/ninja/releases/latest/download/ninja-linux.zip
diff --git a/libcxx/utils/libcxx/test/features.py b/libcxx/utils/libcxx/test/features.py
index 7d6e78de343c5..21e86a44e745b 100644
--- a/libcxx/utils/libcxx/test/features.py
+++ b/libcxx/utils/libcxx/test/features.py
@@ -355,6 +355,14 @@ def _mingwSupportsModules(cfg):
name="has-no-zdump",
when=lambda cfg: runScriptExitCode(cfg, ["zdump --version"]) != 0,
),
+ # Whether the `filecheck` executable is available. Note that this corresponds to
+ # a Python port of LLVM's FileCheck, not LLVM's actual FileCheck program, since
+ # that one requires building parts of LLVM that we don't want to build when merely
+ # testing libc++.
+ Feature(
+ name="has-filecheck",
+ when=lambda cfg: runScriptExitCode(cfg, ["filecheck --version"]) == 0,
+ ),
]
# Deduce and add the test features that that are implied by the #defines in
|
There was a problem hiding this comment.
Drive by comment from a non libc++ contributor, so take my feedback as you will.
Why install a Python version of FileCheck when we could just install/build actual FileCheck? I'd prefer to avoid external dependencies whenever possible, especially when it's a much less battle tested reimplementation of something that we have in tree.
The problem is that AFAICT, LLVM's |
It does require configuring LLVM, but I'm not sure that requires much more than configuring libc++. Looking at it right now, building just FileCheck takes 188 build steps. It looks like it's just LLVM Support and FileCheck itself, which I feel is reasonably small. From what I understand, this is primarily intended to test codegen. For that, we probably want to ensure we're testing with clang so that we have at least some control over the actual codegen which should imply getting FileCheck working is possible. |
libc++ Takes 173 steps (excluding copying headers), so you'd basically be looking at doubling the build time. It's not definitely unreasoable, since libc++ is relatively quick to build, but still a lot worse than 188 steps might sound at first. Anyways, I think the much bigger problem would be that we'd create a circular dependency between the runtimes build and the llvm build.
That's the primary reason, but I'd also really like to add tests for our clang-tidy checks with this. |
| # Whether the `filecheck` executable is available. Note that this corresponds to | ||
| # a Python port of LLVM's FileCheck, not LLVM's actual FileCheck program, since | ||
| # that one requires building parts of LLVM that we don't want to build when merely | ||
| # testing libc++. | ||
| Feature( | ||
| name="has-filecheck", | ||
| when=lambda cfg: runScriptExitCode(cfg, ["filecheck --version"]) == 0, | ||
| ), |
There was a problem hiding this comment.
I think allowing the external version is very reasonable, but IMO we should also look for FileCheck, since folks may very well have that already on their system (e.g. I do).
There was a problem hiding this comment.
The issue though is that we have to standardize on the case we're going to use in the tests. LLVM uses FileCheck and the Python port is filecheck (all lowercase).
It looks like the plan here is to find |
If a user wants to set up If we productized |
Can you elaborate on why this process is fragile? I don't think configuring LLVM/building FileCheck should be more fragile than building libc++.
I don't think this would be too difficult to do. We could even make it installable via pip. |
Fragile in the sense that configuring and building LLVM (or parts of it) is not a simple process. It may not be super difficult to do from the command-line (just use the right command-line invocation), however what's involved under the hood is not simple. For that reason, I envision people potentially running into issues trying to do that and the ergonomics wouldn't be great.
I looked into it in the past and couldn't figure out how to do it because |
Why would that matter when
I don't think it should require any refactoring. Just a bit of packaging infrastructure. I can look into getting a pypi distributon setup. |
4 participants
This patch adds support for filecheck tests in libc++'s test suite. However, it doesn't use LLVM's filecheck program, which requires building a bunch of LLVM utilities. Instead, it installs the Python port of filecheck at https://github.com/AntonLydike/filecheck which supports basically the same functionality.
The test suite still works when filecheck is not available, since tests that use filecheck should be guarded on the
has-filecheckLit feature.This should make it possible to test several things that were previously impossible to test, especially for specific code generation.
Supersedes #65917