commit 19bf826e6e5d05100cb3568e90e48bd3c97d4f22
author Shai Barack <shayba@google.com> Sat Dec 18 20:43:33 2021 -0800
committer Commit Bot <commit-bot@chromium.org> Wed Jan 05 22:04:54 2022 +0000
tree 5216e8e9ce6656b6d902aca60c2048ce1f3c1d9e
parent 281ba2c91861b10fec7407c4b6172ec3d4661243

[testonly] Allow configs to be testonly

Bug: 214
Change-Id: Idec794f14f5755eebc1e3ed879c19b015f6d433b
Reviewed-on: https://gn-review.googlesource.com/c/gn/+/12680
Reviewed-by: David Turner <digit@google.com>
Reviewed-by: Brett Wilson <brettw@chromium.org>
Commit-Queue: Shai Barack <shayba@google.com>

src/gn/config_values_generator.h

diff --git a/src/gn/config_values_generator.h b/src/gn/config_values_generator.h
index fd508d4..d006140 100644
--- a/src/gn/config_values_generator.h
+++ b/src/gn/config_values_generator.h
@@ -41,6 +41,6 @@
   "  Flags: cflags, cflags_c, cflags_cc, cflags_objc, cflags_objcc,\n"     \
   "         asmflags, defines, include_dirs, inputs, ldflags, lib_dirs,\n" \
   "         libs, precompiled_header, precompiled_source, rustflags,\n"    \
-  "         rustenv, swiftflags\n"
+  "         rustenv, swiftflags, testonly\n"
 
 #endif  // TOOLS_GN_CONFIG_VALUES_GENERATOR_H_

src/gn/item.h

diff --git a/src/gn/item.h b/src/gn/item.h
index 2942a66..dd1a606 100644
--- a/src/gn/item.h
+++ b/src/gn/item.h
@@ -38,6 +38,9 @@
   const ParseNode* defined_from() const { return defined_from_; }
   void set_defined_from(const ParseNode* df) { defined_from_ = df; }
 
+  bool testonly() const { return testonly_; }
+  void set_testonly(bool value) { testonly_ = value; }
+
   Visibility& visibility() { return visibility_; }
   const Visibility& visibility() const { return visibility_; }
 
@@ -69,11 +72,14 @@
   virtual bool OnResolved(Err* err);
 
  private:
+  bool CheckTestonly(Err* err) const;
+
   const Settings* settings_;
   Label label_;
   SourceFileSet build_dependency_files_;
   const ParseNode* defined_from_;
 
+  bool testonly_ = false;
   Visibility visibility_;
 };
 

src/gn/target.cc

diff --git a/src/gn/target.cc b/src/gn/target.cc
index da6b7e4..ded575a 100644
--- a/src/gn/target.cc
+++ b/src/gn/target.cc
@@ -43,7 +43,7 @@
   }
 }
 
-Err MakeTestOnlyError(const Target* from, const Target* to) {
+Err MakeTestOnlyError(const Item* from, const Item* to) {
   return Err(
       from->defined_from(), "Test-only dependency not allowed.",
       from->label().GetUserVisibleName(false) +
@@ -1142,6 +1142,16 @@
     }
   }
 
+  // Verify no configs have "testonly" set.
+  for (ConfigValuesIterator iter(this); !iter.done(); iter.Next()) {
+    if (const Config* config = iter.GetCurrentConfig()) {
+      if (config->testonly()) {
+        *err = MakeTestOnlyError(this, config);
+        return false;
+      }
+    }
+  }
+
   return true;
 }
 

src/gn/target.h

diff --git a/src/gn/target.h b/src/gn/target.h
index 59fe806..d88f0a5 100644
--- a/src/gn/target.h
+++ b/src/gn/target.h
@@ -214,9 +214,6 @@
   }
   std::vector<std::string>& walk_keys() { return generated_file().walk_keys_; }
 
-  bool testonly() const { return testonly_; }
-  void set_testonly(bool value) { testonly_ = value; }
-
   OutputFile write_runtime_deps_output() const {
     return write_runtime_deps_output_;
   }
@@ -474,7 +471,6 @@
   FileList public_headers_;
   bool check_includes_ = true;
   bool complete_static_lib_ = false;
-  bool testonly_ = false;
   std::vector<std::string> data_;
   std::unique_ptr<BundleData> bundle_data_;
   OutputFile write_runtime_deps_output_;

src/gn/target_unittest.cc

diff --git a/src/gn/target_unittest.cc b/src/gn/target_unittest.cc
index f8cbcd1..b4ea699 100644
--- a/src/gn/target_unittest.cc
+++ b/src/gn/target_unittest.cc
@@ -628,6 +628,31 @@
   ASSERT_FALSE(product.OnResolved(&err));
 }
 
+// Configs can be testonly too.
+// Repeat the testonly test with a config.
+TEST_F(TargetTest, TestonlyConfig) {
+  TestWithScope setup;
+  Err err;
+
+  // "testconfig" is a test-only config.
+  Config testconfig(setup.settings(), Label(SourceDir("//test/"), "config"));
+  testconfig.set_testonly(true);
+  testconfig.visibility().SetPublic();
+  ASSERT_TRUE(testconfig.OnResolved(&err));
+
+  // "test" is a test-only executable that uses testconfig, this is OK.
+  TestTarget test(setup, "//test:test", Target::EXECUTABLE);
+  test.set_testonly(true);
+  test.configs().push_back(LabelConfigPair(&testconfig));
+  ASSERT_TRUE(test.OnResolved(&err));
+
+  // "product" is a non-test that uses testconfig. This should fail.
+  TestTarget product(setup, "//app:product", Target::EXECUTABLE);
+  product.set_testonly(false);
+  product.configs().push_back(LabelConfigPair(&testconfig));
+  ASSERT_FALSE(product.OnResolved(&err));
+}
+
 TEST_F(TargetTest, PublicConfigs) {
   TestWithScope setup;
   Err err;