Skip to content

[clang-doc] Avoid reading files in unit tests #141269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 23, 2025

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented May 23, 2025

In #138062 it was brought up that this was an anti-pattern. We'll need
to Migrate all of the mustache unittests that need to read template
files to lit tests, and disable them until tool support lands.

In #138062 it was brought up that this was an anti-pattern. We'll need
to Migrate all of the mustache unittests to lit tests, and disable them
until tool support lands.
Copy link
Contributor Author

ilovepi commented May 23, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@ilovepi ilovepi marked this pull request as ready for review May 23, 2025 18:03
@ilovepi ilovepi requested a review from nico May 23, 2025 18:04
@llvmbot
Copy link
Member

llvmbot commented May 23, 2025

@llvm/pr-subscribers-clang-tools-extra

Author: Paul Kirth (ilovepi)

Changes

In #138062 it was brought up that this was an anti-pattern. We'll need
to Migrate all of the mustache unittests to lit tests, and disable them
until tool support lands.


Full diff: https://github.com/llvm/llvm-project/pull/141269.diff

1 Files Affected:

  • (modified) clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp (+4-2)
diff --git a/clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp
index 70491f0754b3d..4c8cf4fa7e460 100644
--- a/clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp
+++ b/clang-tools-extra/unittests/clang-doc/HTMLMustacheGeneratorTest.cpp
@@ -91,10 +91,12 @@ TEST(HTMLMustacheGeneratorTest, generateDocs) {
   unittest::TempDir RootTestDirectory("generateDocsTest", /*Unique=*/true);
   CDCtx.OutDirectory = RootTestDirectory.path();
 
-  getMustacheHtmlFiles(CLANG_DOC_TEST_ASSET_DIR, CDCtx);
+  // FIXME: We can't read files during unit tests. Migrate to lit once
+  // tool support lands.
+  // getMustacheHtmlFiles(CLANG_DOC_TEST_ASSET_DIR, CDCtx);
 
   EXPECT_THAT_ERROR(G->generateDocs(RootTestDirectory.path(), {}, CDCtx),
-                    Succeeded())
+                    Failed())
       << "Failed to generate docs.";
 }
 

Copy link
Contributor Author

ilovepi commented May 23, 2025

Merge activity

  • May 23, 6:45 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 23, 6:47 PM UTC: @ilovepi merged this pull request with Graphite.

@ilovepi ilovepi merged commit 524ef16 into main May 23, 2025
15 checks passed
@ilovepi ilovepi deleted the users/ilovepi/clang-doc-disable-file-test branch May 23, 2025 18:47
@nico
Copy link
Contributor

nico commented May 23, 2025

Thanks! Can we stop generating config.h from config.h.cmake again too? That should no longer be needed now.

sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Jun 3, 2025
In llvm#138062 it was brought up that this was an anti-pattern. We'll need
to Migrate all of the mustache unittests that need to read template 
files to lit tests, and disable them until tool support lands.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants