From 1ea01e3c59ce8f7e10aa1c893f1dd67b6c5d4b63 Mon Sep 17 00:00:00 2001 From: mrziwang Date: Fri, 12 Jul 2024 12:26:34 -0700 Subject: [PATCH] Use OutputFilesProvider in module_test In the context of incremental soong, the output files inter-module-communication will be through OutputFilesProvider. The OutputFileProducer interface will be deprecated. Test: CI Bug: 339477385 Change-Id: Iaf0b039b17d34e1a696f2c87df2f0db39307fc6d --- android/module.go | 21 +++---- android/module_test.go | 140 +++++++++++++++++++++++++++++------------ 2 files changed, 107 insertions(+), 54 deletions(-) diff --git a/android/module.go b/android/module.go index 2fe822f26..d9b85455a 100644 --- a/android/module.go +++ b/android/module.go @@ -2491,18 +2491,9 @@ func outputFilesForModule(ctx PathContext, module blueprint.Module, tag string) if outputFilesFromProvider != nil || err != OutputFilesProviderNotSet { return outputFilesFromProvider, err } - // TODO: add error when outputFilesFromProvider and err are both nil after - // OutputFileProducer and SourceFileProducer are deprecated. - if outputFileProducer, ok := module.(OutputFileProducer); ok { - paths, err := outputFileProducer.OutputFiles(tag) - if err != nil { - return nil, fmt.Errorf("failed to get output file from module %q at tag %q: %s", - pathContextName(ctx, module), tag, err.Error()) - } - return paths, nil - } else if sourceFileProducer, ok := module.(SourceFileProducer); ok { + if sourceFileProducer, ok := module.(SourceFileProducer); ok { if tag != "" { - return nil, fmt.Errorf("module %q is a SourceFileProducer, not an OutputFileProducer, and so does not support tag %q", pathContextName(ctx, module), tag) + return nil, fmt.Errorf("module %q is a SourceFileProducer, which does not support tag %q", pathContextName(ctx, module), tag) } paths := sourceFileProducer.Srcs() return paths, nil @@ -2521,7 +2512,12 @@ func outputFilesForModuleFromProvider(ctx PathContext, module blueprint.Module, var outputFiles OutputFilesInfo fromProperty := false - if mctx, isMctx := ctx.(ModuleContext); isMctx { + type OutputFilesProviderModuleContext interface { + OtherModuleProviderContext + Module() Module + } + + if mctx, isMctx := ctx.(OutputFilesProviderModuleContext); isMctx { if mctx.Module() != module { outputFiles, _ = OtherModuleProvider(mctx, module, OutputFilesProvider) } else { @@ -2535,7 +2531,6 @@ func outputFilesForModuleFromProvider(ctx PathContext, module blueprint.Module, // TODO: Add a check for skipped context if outputFiles.isEmpty() { - // TODO: Add a check for param module not having OutputFilesProvider set return nil, OutputFilesProviderNotSet } diff --git a/android/module_test.go b/android/module_test.go index 1f3db5c5c..922ea21fe 100644 --- a/android/module_test.go +++ b/android/module_test.go @@ -935,31 +935,54 @@ func TestSetAndroidMkEntriesWithTestOptions(t *testing.T) { } } -type fakeBlueprintModule struct{} - -func (fakeBlueprintModule) Name() string { return "foo" } - -func (fakeBlueprintModule) GenerateBuildActions(blueprint.ModuleContext) {} - type sourceProducerTestModule struct { - fakeBlueprintModule - source Path + ModuleBase + props struct { + // A represents the source file + A string + } } -func (s sourceProducerTestModule) Srcs() Paths { return Paths{s.source} } - -type outputFileProducerTestModule struct { - fakeBlueprintModule - output map[string]Path - error map[string]error +func sourceProducerTestModuleFactory() Module { + module := &sourceProducerTestModule{} + module.AddProperties(&module.props) + InitAndroidModule(module) + return module } -func (o outputFileProducerTestModule) OutputFiles(tag string) (Paths, error) { - return PathsIfNonNil(o.output[tag]), o.error[tag] +func (s sourceProducerTestModule) GenerateAndroidBuildActions(ModuleContext) {} + +func (s sourceProducerTestModule) Srcs() Paths { return PathsForTesting(s.props.A) } + +type outputFilesTestModule struct { + ModuleBase + props struct { + // A represents the tag + A string + // B represents the output file for tag A + B string + } +} + +func outputFilesTestModuleFactory() Module { + module := &outputFilesTestModule{} + module.AddProperties(&module.props) + InitAndroidModule(module) + return module +} + +func (o outputFilesTestModule) GenerateAndroidBuildActions(ctx ModuleContext) { + if o.props.A != "" || o.props.B != "" { + ctx.SetOutputFiles(PathsForTesting(o.props.B), o.props.A) + } + // This is to simulate the case that some module uses an object to set its + // OutputFilesProvider, but the object itself is empty. + ctx.SetOutputFiles(Paths{}, "missing") } type pathContextAddMissingDependenciesWrapper struct { PathContext + OtherModuleProviderContext missingDeps []string } @@ -970,52 +993,87 @@ func (p *pathContextAddMissingDependenciesWrapper) OtherModuleName(module bluepr return module.Name() } +func (p *pathContextAddMissingDependenciesWrapper) Module() Module { return nil } + func TestOutputFileForModule(t *testing.T) { testcases := []struct { name string - module blueprint.Module + bp string tag string - env map[string]string - config func(*config) expected string missingDeps []string + env map[string]string + config func(*config) }{ { - name: "SourceFileProducer", - module: &sourceProducerTestModule{source: PathForTesting("foo.txt")}, - expected: "foo.txt", + name: "SourceFileProducer", + bp: `spt_module { + name: "test_module", + a: "spt.txt", + } + `, + tag: "", + expected: "spt.txt", }, { - name: "OutputFileProducer", - module: &outputFileProducerTestModule{output: map[string]Path{"": PathForTesting("foo.txt")}}, - expected: "foo.txt", + name: "OutputFileProviderEmptyStringTag", + bp: `oft_module { + name: "test_module", + a: "", + b: "empty.txt", + } + `, + tag: "", + expected: "empty.txt", }, { - name: "OutputFileProducer_tag", - module: &outputFileProducerTestModule{output: map[string]Path{"foo": PathForTesting("foo.txt")}}, + name: "OutputFileProviderTag", + bp: `oft_module { + name: "test_module", + a: "foo", + b: "foo.txt", + } + `, tag: "foo", expected: "foo.txt", }, { - name: "OutputFileProducer_AllowMissingDependencies", + name: "OutputFileAllowMissingDependencies", + bp: `oft_module { + name: "test_module", + } + `, + tag: "missing", + expected: "missing_output_file/test_module", + missingDeps: []string{"test_module"}, config: func(config *config) { config.TestProductVariables.Allow_missing_dependencies = boolPtr(true) }, - module: &outputFileProducerTestModule{}, - missingDeps: []string{"foo"}, - expected: "missing_output_file/foo", }, } + for _, tt := range testcases { - config := TestConfig(buildDir, tt.env, "", nil) - if tt.config != nil { - tt.config(config.config) - } - ctx := &pathContextAddMissingDependenciesWrapper{ - PathContext: PathContextForTesting(config), - } - got := OutputFileForModule(ctx, tt.module, tt.tag) - AssertPathRelativeToTopEquals(t, "expected source path", tt.expected, got) - AssertArrayString(t, "expected missing deps", tt.missingDeps, ctx.missingDeps) + t.Run(tt.name, func(t *testing.T) { + result := GroupFixturePreparers( + PrepareForTestWithDefaults, + FixtureRegisterWithContext(func(ctx RegistrationContext) { + ctx.RegisterModuleType("spt_module", sourceProducerTestModuleFactory) + ctx.RegisterModuleType("oft_module", outputFilesTestModuleFactory) + }), + FixtureWithRootAndroidBp(tt.bp), + ).RunTest(t) + + config := TestConfig(buildDir, tt.env, tt.bp, nil) + if tt.config != nil { + tt.config(config.config) + } + ctx := &pathContextAddMissingDependenciesWrapper{ + PathContext: PathContextForTesting(config), + OtherModuleProviderContext: result.TestContext.OtherModuleProviderAdaptor(), + } + got := OutputFileForModule(ctx, result.ModuleForTests("test_module", "").Module(), tt.tag) + AssertPathRelativeToTopEquals(t, "expected output path", tt.expected, got) + AssertArrayString(t, "expected missing deps", tt.missingDeps, ctx.missingDeps) + }) } }