From 4814bb814a15ce7cb23df06e2e573a4e23c9cce0 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Mon, 29 Mar 2021 01:55:10 +0100 Subject: [PATCH 1/2] Remove FixtureFactory Bug: 182885307 Test: m nothing Change-Id: I644db99cc6905f544d3e7479b435be26dbf6c59b --- android/fixture.go | 134 +++------------------------------------------ 1 file changed, 8 insertions(+), 126 deletions(-) diff --git a/android/fixture.go b/android/fixture.go index 303c95c2e..883647c0a 100644 --- a/android/fixture.go +++ b/android/fixture.go @@ -26,16 +26,9 @@ import ( // Fixture // ======= // These determine the environment within which a test can be run. Fixtures are mutable and are -// created by FixtureFactory instances and mutated by FixturePreparer instances. They are created by -// first creating a base Fixture (which is essentially empty) and then applying FixturePreparer -// instances to it to modify the environment. -// -// FixtureFactory (deprecated) -// =========================== -// These are responsible for creating fixtures. Factories are immutable and are intended to be -// initialized once and reused to create multiple fixtures. Each factory has a list of fixture -// preparers that prepare a fixture for running a test. Factories can also be used to create other -// factories by extending them with additional fixture preparers. +// created and mutated by FixturePreparer instances. They are created by first creating a base +// Fixture (which is essentially empty) and then applying FixturePreparer instances to it to modify +// the environment. // // FixturePreparer // =============== @@ -169,77 +162,6 @@ import ( // PrepareForApex, // ) // -// // FixtureFactory instances have been deprecated, this remains for informational purposes to -// // help explain some of the existing code but will be removed along with FixtureFactory. -// -// var javaFixtureFactory = android.NewFixtureFactory( -// PrepareForIntegrationTestWithJava, -// FixtureRegisterWithContext(func(ctx android.RegistrationContext) { -// ctx.RegisterModuleType("test_module", testModule) -// }), -// javaMockFS.AddToFixture(), -// ... -// } -// -// func TestJavaStuff(t *testing.T) { -// result := javaFixtureFactory.RunTest(t, -// android.FixtureWithRootAndroidBp(`java_library {....}`), -// android.MockFS{...}.AddToFixture(), -// ) -// ... test result ... -// } -// -// package cc -// var PrepareForTestWithCC = GroupFixturePreparers( -// android.PrepareForArchMutator, -// android.prepareForPrebuilts, -// FixtureRegisterWithContext(RegisterRequiredBuildComponentsForTest), -// ... -// ) -// -// package apex -// -// var PrepareForApex = GroupFixturePreparers( -// ... -// ) -// -// Use modules and mutators from java, cc and apex. Any duplicate preparers (like -// android.PrepareForArchMutator) will be automatically deduped. -// -// var apexFixtureFactory = android.NewFixtureFactory( -// PrepareForJava, -// PrepareForCC, -// PrepareForApex, -// ) - -// Factory for Fixture objects. -// -// This is configured with a set of FixturePreparer objects that are used to -// initialize each Fixture instance this creates. -// -// deprecated: Use FixturePreparer instead. -type FixtureFactory interface { - FixturePreparer -} - -// Create a new FixtureFactory that will apply the supplied preparers. -// -// The buildDirSupplier is a pointer to the package level buildDir variable that is initialized by -// the package level setUp method. It has to be a pointer to the variable as the variable will not -// have been initialized at the time the factory is created. If it is nil then a test specific -// temporary directory will be created instead. -// -// deprecated: The functionality provided by FixtureFactory will be merged into FixturePreparer -func NewFixtureFactory(buildDirSupplier *string, preparers ...FixturePreparer) FixtureFactory { - f := &fixtureFactory{ - buildDirSupplier: buildDirSupplier, - compositeFixturePreparer: compositeFixturePreparer{ - preparers: dedupAndFlattenPreparers(nil, preparers), - }, - } - f.initBaseFixturePreparer(f) - return f -} // A set of mock files to add to the mock file system. type MockFS map[string][]byte @@ -447,9 +369,9 @@ type FixturePreparer interface { // Creates a copy of this instance and adds some additional preparers. // - // Before the preparers are used they are combined with the preparers provided when the factory - // was created, any groups of preparers are flattened, and the list is deduped so that each - // preparer is only used once. See the file documentation in android/fixture.go for more details. + // Before the preparers are used they are combined with the current preparer, any groups of + // preparers are flattened, and the list is deduped so that each preparer is only used once. See + // the file documentation in android/fixture.go for more details. // // deprecated: Use GroupFixturePreparers() instead. Extend(preparers ...FixturePreparer) FixturePreparer @@ -782,46 +704,6 @@ func (b *baseFixturePreparer) RunTestWithConfig(t *testing.T, config Config) *Te return fixture.RunTest() } -var _ FixtureFactory = (*fixtureFactory)(nil) - -type fixtureFactory struct { - compositeFixturePreparer - - buildDirSupplier *string -} - -// Override to preserve the buildDirSupplier. -func (f *fixtureFactory) Extend(preparers ...FixturePreparer) FixturePreparer { - // If there is no buildDirSupplier then just use the default implementation. - if f.buildDirSupplier == nil { - return f.baseFixturePreparer.Extend(preparers...) - } - - all := dedupAndFlattenPreparers(f.preparers, preparers) - - // Create a new factory which uses the same buildDirSupplier as the previous one. - extendedFactory := &fixtureFactory{ - buildDirSupplier: f.buildDirSupplier, - compositeFixturePreparer: compositeFixturePreparer{ - preparers: all, - }, - } - extendedFactory.initBaseFixturePreparer(extendedFactory) - return extendedFactory -} - -func (f *fixtureFactory) Fixture(t *testing.T) Fixture { - // If there is no buildDirSupplier then just use the default implementation. - if f.buildDirSupplier == nil { - return f.baseFixturePreparer.Fixture(t) - } - - // Retrieve the buildDir from the supplier. - buildDir := *f.buildDirSupplier - - return createFixture(t, buildDir, f.preparers) -} - type fixture struct { // The preparers used to create this fixture. preparers []*simpleFixturePreparer @@ -936,10 +818,10 @@ func (r *TestResult) NormalizePathsForTesting(paths Paths) []string { // that produced this result. // // e.g. assuming that this result was created by running: -// factory.Extend(preparer1, preparer2).RunTest(t, preparer3, preparer4) +// GroupFixturePreparers(preparer1, preparer2, preparer3).RunTest(t) // // Then this method will be equivalent to running: -// GroupFixturePreparers(preparer1, preparer2, preparer3, preparer4) +// GroupFixturePreparers(preparer1, preparer2, preparer3) // // This is intended for use by tests whose output is Android.bp files to verify that those files // are valid, e.g. tests of the snapshots produced by the sdk module type. From 79abe57f534771027ca77ea01a274d306a82f394 Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Mon, 29 Mar 2021 02:16:14 +0100 Subject: [PATCH 2/2] Remove FixturePreparer.Extend() Use GroupFixturePreparers instead. Bug: 182885307 Test: m nothing Change-Id: Idc01d3cc5a57576a4cf417e9105d1ab851126e10 --- android/fixture.go | 16 +--------------- android/fixture_test.go | 2 +- genrule/genrule_test.go | 3 ++- java/hiddenapi_singleton_test.go | 24 ++++++++++++++++-------- java/sdk_test.go | 6 ++++-- rust/project_json_test.go | 7 ++++--- 6 files changed, 28 insertions(+), 30 deletions(-) diff --git a/android/fixture.go b/android/fixture.go index 883647c0a..5fc668a8b 100644 --- a/android/fixture.go +++ b/android/fixture.go @@ -367,15 +367,6 @@ type FixturePreparer interface { // Return the flattened and deduped list of simpleFixturePreparer pointers. list() []*simpleFixturePreparer - // Creates a copy of this instance and adds some additional preparers. - // - // Before the preparers are used they are combined with the current preparer, any groups of - // preparers are flattened, and the list is deduped so that each preparer is only used once. See - // the file documentation in android/fixture.go for more details. - // - // deprecated: Use GroupFixturePreparers() instead. - Extend(preparers ...FixturePreparer) FixturePreparer - // Create a Fixture. Fixture(t *testing.T) Fixture @@ -656,17 +647,12 @@ func (b *baseFixturePreparer) initBaseFixturePreparer(self FixturePreparer) { b.self = self } -func (b *baseFixturePreparer) Extend(preparers ...FixturePreparer) FixturePreparer { - all := dedupAndFlattenPreparers(b.self.list(), preparers) - return newFixturePreparer(all) -} - func (b *baseFixturePreparer) Fixture(t *testing.T) Fixture { return createFixture(t, t.TempDir(), b.self.list()) } func (b *baseFixturePreparer) ExtendWithErrorHandler(errorHandler FixtureErrorHandler) FixturePreparer { - return b.self.Extend(newSimpleFixturePreparer(func(fixture *fixture) { + return GroupFixturePreparers(b.self, newSimpleFixturePreparer(func(fixture *fixture) { fixture.errorHandler = errorHandler })) } diff --git a/android/fixture_test.go b/android/fixture_test.go index 8f0471560..5b810e0b7 100644 --- a/android/fixture_test.go +++ b/android/fixture_test.go @@ -41,7 +41,7 @@ func TestFixtureDedup(t *testing.T) { group := GroupFixturePreparers(preparer1, preparer2, preparer1, preparer1Then2) - extension := group.Extend(preparer4, preparer2) + extension := GroupFixturePreparers(group, preparer4, preparer2) GroupFixturePreparers(extension, preparer1, preparer2, preparer2Then1, preparer3).Fixture(t) diff --git a/genrule/genrule_test.go b/genrule/genrule_test.go index 2ee456d56..3f1e9f3b9 100644 --- a/genrule/genrule_test.go +++ b/genrule/genrule_test.go @@ -670,7 +670,8 @@ func TestGenruleAllowMissingDependencies(t *testing.T) { cmd: "cat $(in) > $(out)", } ` - result := prepareForGenRuleTest.Extend( + result := android.GroupFixturePreparers( + prepareForGenRuleTest, android.FixtureModifyConfigAndContext( func(config android.Config, ctx *android.TestContext) { config.TestProductVariables.Allow_missing_dependencies = proptools.BoolPtr(true) diff --git a/java/hiddenapi_singleton_test.go b/java/hiddenapi_singleton_test.go index dc4e8aaff..5c449e506 100644 --- a/java/hiddenapi_singleton_test.go +++ b/java/hiddenapi_singleton_test.go @@ -39,7 +39,8 @@ var hiddenApiFixtureFactory = android.GroupFixturePreparers( prepareForJavaTest, PrepareForTestWithHiddenApiBuildComponents) func TestHiddenAPISingleton(t *testing.T) { - result := hiddenApiFixtureFactory.Extend( + result := android.GroupFixturePreparers( + hiddenApiFixtureFactory, fixtureSetBootJarsProductVariable("platform:foo"), ).RunTestWithBp(t, ` java_library { @@ -56,7 +57,8 @@ func TestHiddenAPISingleton(t *testing.T) { } func TestHiddenAPIIndexSingleton(t *testing.T) { - result := hiddenApiFixtureFactory.Extend( + result := android.GroupFixturePreparers( + hiddenApiFixtureFactory, PrepareForTestWithJavaSdkLibraryFiles, FixtureWithLastReleaseApis("bar"), fixtureSetBootJarsProductVariable("platform:foo", "platform:bar"), @@ -115,7 +117,8 @@ func TestHiddenAPISingletonWithSourceAndPrebuiltPreferredButNoDex(t *testing.T) " replaced by the prebuilt module \"prebuilt_foo\" but unfortunately it does not provide a" + " suitable boot dex jar" - hiddenApiFixtureFactory.Extend( + android.GroupFixturePreparers( + hiddenApiFixtureFactory, fixtureSetBootJarsProductVariable("platform:foo"), ).ExtendWithErrorHandler(android.FixtureExpectsAtLeastOneErrorMatchingPattern(expectedErrorMessage)). RunTestWithBp(t, ` @@ -134,7 +137,8 @@ func TestHiddenAPISingletonWithSourceAndPrebuiltPreferredButNoDex(t *testing.T) } func TestHiddenAPISingletonWithPrebuilt(t *testing.T) { - result := hiddenApiFixtureFactory.Extend( + result := android.GroupFixturePreparers( + hiddenApiFixtureFactory, fixtureSetBootJarsProductVariable("platform:foo"), ).RunTestWithBp(t, ` java_import { @@ -151,7 +155,8 @@ func TestHiddenAPISingletonWithPrebuilt(t *testing.T) { } func TestHiddenAPISingletonWithPrebuiltUseSource(t *testing.T) { - result := hiddenApiFixtureFactory.Extend( + result := android.GroupFixturePreparers( + hiddenApiFixtureFactory, fixtureSetBootJarsProductVariable("platform:foo"), ).RunTestWithBp(t, ` java_library { @@ -178,7 +183,8 @@ func TestHiddenAPISingletonWithPrebuiltUseSource(t *testing.T) { } func TestHiddenAPISingletonWithPrebuiltOverrideSource(t *testing.T) { - result := hiddenApiFixtureFactory.Extend( + result := android.GroupFixturePreparers( + hiddenApiFixtureFactory, fixtureSetBootJarsProductVariable("platform:foo"), ).RunTestWithBp(t, ` java_library { @@ -236,7 +242,8 @@ func TestHiddenAPISingletonSdks(t *testing.T) { } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result := hiddenApiFixtureFactory.Extend( + result := android.GroupFixturePreparers( + hiddenApiFixtureFactory, tc.preparer, android.FixtureModifyProductVariables(func(variables android.FixtureProductVariables) { variables.Always_use_prebuilt_sdks = proptools.BoolPtr(tc.unbundledBuild) @@ -286,7 +293,8 @@ func TestHiddenAPISingletonWithPrebuiltCsvFile(t *testing.T) { // Where to find the prebuilt hiddenapi files: prebuiltHiddenApiDir := "path/to/prebuilt/hiddenapi" - result := hiddenApiFixtureFactory.Extend( + result := android.GroupFixturePreparers( + hiddenApiFixtureFactory, fixtureSetBootJarsProductVariable("platform:foo"), fixtureSetPrebuiltHiddenApiDirProductVariable(&prebuiltHiddenApiDir), ).RunTestWithBp(t, ` diff --git a/java/sdk_test.go b/java/sdk_test.go index e1ec41bdc..2b1846592 100644 --- a/java/sdk_test.go +++ b/java/sdk_test.go @@ -402,14 +402,16 @@ func TestClasspath(t *testing.T) { // Test again with PLATFORM_VERSION_CODENAME=REL, javac -source 8 -target 8 t.Run("REL + Java language level 8", func(t *testing.T) { - result := fixtureFactory.Extend(prepareWithPlatformVersionRel).RunTestWithBp(t, bpJava8) + result := android.GroupFixturePreparers( + fixtureFactory, prepareWithPlatformVersionRel).RunTestWithBp(t, bpJava8) checkClasspath(t, result, true /* isJava8 */) }) // Test again with PLATFORM_VERSION_CODENAME=REL, javac -source 9 -target 9 t.Run("REL + Java language level 9", func(t *testing.T) { - result := fixtureFactory.Extend(prepareWithPlatformVersionRel).RunTestWithBp(t, bp) + result := android.GroupFixturePreparers( + fixtureFactory, prepareWithPlatformVersionRel).RunTestWithBp(t, bp) checkClasspath(t, result, false /* isJava8 */) }) diff --git a/rust/project_json_test.go b/rust/project_json_test.go index 7af463528..09d30dbde 100644 --- a/rust/project_json_test.go +++ b/rust/project_json_test.go @@ -28,9 +28,10 @@ import ( // testProjectJson run the generation of rust-project.json. It returns the raw // content of the generated file. func testProjectJson(t *testing.T, bp string) []byte { - result := prepareForRustTest. - Extend(android.FixtureMergeEnv(map[string]string{"SOONG_GEN_RUST_PROJECT": "1"})). - RunTestWithBp(t, bp) + result := android.GroupFixturePreparers( + prepareForRustTest, + android.FixtureMergeEnv(map[string]string{"SOONG_GEN_RUST_PROJECT": "1"}), + ).RunTestWithBp(t, bp) // The JSON file is generated via WriteFileToOutputDir. Therefore, it // won't appear in the Output of the TestingSingleton. Manually verify