diff --git a/sdk/cc_sdk_test.go b/sdk/cc_sdk_test.go index 9626a047e..6c02bf755 100644 --- a/sdk/cc_sdk_test.go +++ b/sdk/cc_sdk_test.go @@ -1816,7 +1816,10 @@ myinclude/Test.h -> include/myinclude/Test.h .intermediates/mynativelib/android_arm64_armv8-a_static/mynativelib.a -> arm64/lib/mynativelib.a .intermediates/mynativelib/android_arm64_armv8-a_shared/mynativelib.so -> arm64/lib/mynativelib.so .intermediates/mynativelib/android_arm_armv7-a-neon_static/mynativelib.a -> arm/lib/mynativelib.a -.intermediates/mynativelib/android_arm_armv7-a-neon_shared/mynativelib.so -> arm/lib/mynativelib.so`), +.intermediates/mynativelib/android_arm_armv7-a-neon_shared/mynativelib.so -> arm/lib/mynativelib.so +`), + // TODO(b/183315522): Remove this and fix the issue. + snapshotTestErrorHandler(checkSnapshotPreferredWithSource, android.FixtureExpectsAtLeastOneErrorMatchingPattern(`\Qunrecognized property "arch.arm.shared.export_include_dirs"\E`)), ) } @@ -2664,6 +2667,11 @@ func TestNoSanitizerMembers(t *testing.T) { } `) + // Mixing the snapshot with the source (irrespective of which one is preferred) causes a problem + // due to missing variants. + // TODO(b/183204176): Remove this and fix the cause. + snapshotWithSourceErrorHandler := android.FixtureExpectsAtLeastOneErrorMatchingPattern(`\QReplaceDependencies could not find identical variant {os:android,image:,arch:arm64_armv8-a,sdk:,link:shared,version:} for module mynativelib\E`) + CheckSnapshot(t, result, "mysdk", "", checkUnversionedAndroidBpContents(` // This is auto-generated. DO NOT EDIT. @@ -2688,6 +2696,9 @@ cc_prebuilt_library_shared { checkAllCopyRules(` myinclude/Test.h -> include/myinclude/Test.h arm64/include/Arm64Test.h -> arm64/include/arm64/include/Arm64Test.h -.intermediates/mynativelib/android_arm_armv7-a-neon_shared/mynativelib.so -> arm/lib/mynativelib.so`), +.intermediates/mynativelib/android_arm_armv7-a-neon_shared/mynativelib.so -> arm/lib/mynativelib.so +`), + snapshotTestErrorHandler(checkSnapshotWithSourcePreferred, snapshotWithSourceErrorHandler), + snapshotTestErrorHandler(checkSnapshotPreferredWithSource, snapshotWithSourceErrorHandler), ) } diff --git a/sdk/compat_config_sdk_test.go b/sdk/compat_config_sdk_test.go index dffc02ade..1b62885bd 100644 --- a/sdk/compat_config_sdk_test.go +++ b/sdk/compat_config_sdk_test.go @@ -66,5 +66,30 @@ prebuilt_platform_compat_config { checkAllCopyRules(` .intermediates/myconfig/android_common/myconfig_meta.xml -> compat_configs/myconfig/myconfig_meta.xml `), + snapshotTestChecker(checkSnapshotWithoutSource, + func(t *testing.T, result *android.TestResult) { + // Make sure that the snapshot metadata is collated by the platform compat config singleton. + java.CheckMergedCompatConfigInputs(t, result, "snapshot module", "snapshot/compat_configs/myconfig/myconfig_meta.xml") + }), + + snapshotTestChecker(checkSnapshotWithSourcePreferred, + func(t *testing.T, result *android.TestResult) { + // Make sure that the snapshot metadata is collated by the platform compat config singleton. + java.CheckMergedCompatConfigInputs(t, result, "snapshot module", + "out/soong/.intermediates/myconfig/android_common/myconfig_meta.xml", + // TODO(b/182402754): Remove this as only the config file from the preferred module should be used. + "snapshot/compat_configs/myconfig/myconfig_meta.xml", + ) + }), + + snapshotTestChecker(checkSnapshotPreferredWithSource, + func(t *testing.T, result *android.TestResult) { + // Make sure that the snapshot metadata is collated by the platform compat config singleton. + java.CheckMergedCompatConfigInputs(t, result, "snapshot module", + "out/soong/.intermediates/myconfig/android_common/myconfig_meta.xml", + // TODO(b/182402754): Remove this as only the config file from the preferred module should be used. + "snapshot/compat_configs/myconfig/myconfig_meta.xml", + ) + }), ) } diff --git a/sdk/java_sdk_test.go b/sdk/java_sdk_test.go index fa0eb3fd5..6f78ded35 100644 --- a/sdk/java_sdk_test.go +++ b/sdk/java_sdk_test.go @@ -48,42 +48,17 @@ func TestSdkDependsOnSourceEvenWhenPrebuiltPreferred(t *testing.T) { system_modules: "none", sdk_version: "none", } - - java_import { - name: "sdkmember", - prefer: true, - jars: ["prebuilt.jar"], - } `) // Make sure that the mysdk module depends on "sdkmember" and not "prebuilt_sdkmember". - java.CheckModuleDependencies(t, result.TestContext, "mysdk", "android_common", []string{"sdkmember"}) + sdkChecker := func(t *testing.T, result *android.TestResult) { + java.CheckModuleDependencies(t, result.TestContext, "mysdk", "android_common", []string{"sdkmember"}) + } CheckSnapshot(t, result, "mysdk", "", - checkAndroidBpContents(`// This is auto-generated. DO NOT EDIT. - -java_import { - name: "mysdk_sdkmember@current", - sdk_member_name: "sdkmember", - visibility: ["//visibility:public"], - apex_available: ["//apex_available:platform"], - jars: ["java/sdkmember.jar"], -} - -java_import { - name: "sdkmember", - prefer: false, - visibility: ["//visibility:public"], - apex_available: ["//apex_available:platform"], - jars: ["java/sdkmember.jar"], -} - -sdk_snapshot { - name: "mysdk@current", - visibility: ["//visibility:public"], - java_header_libs: ["mysdk_sdkmember@current"], -} -`)) + snapshotTestChecker(checkSnapshotWithSourcePreferred, sdkChecker), + snapshotTestChecker(checkSnapshotPreferredWithSource, sdkChecker), + ) } func TestBasicSdkWithJavaLibrary(t *testing.T) { diff --git a/sdk/sdk.go b/sdk/sdk.go index e561529b8..b60fb184f 100644 --- a/sdk/sdk.go +++ b/sdk/sdk.go @@ -486,6 +486,15 @@ func sdkDepsReplaceMutator(mctx android.BottomUpMutatorContext) { // sdk containing sdkmember. memberName := versionedSdkMember.MemberName() + // Convert a panic into a normal error to allow it to be more easily tested for. This is a + // temporary workaround, once http://b/183204176 has been fixed this can be removed. + // TODO(b/183204176): Remove this after fixing. + defer func() { + if r := recover(); r != nil { + mctx.ModuleErrorf("%s", r) + } + }() + // Replace dependencies on sdkmember with a dependency on the current module which // is a versioned prebuilt of the sdkmember if required. mctx.ReplaceDependenciesIf(memberName, func(from blueprint.Module, tag blueprint.DependencyTag, to blueprint.Module) bool { diff --git a/sdk/testing.go b/sdk/testing.go index ba40f672b..f2538145b 100644 --- a/sdk/testing.go +++ b/sdk/testing.go @@ -125,6 +125,7 @@ func getSdkSnapshotBuildInfo(t *testing.T, result *android.TestResult, sdk *sdk) androidBpContents: sdk.GetAndroidBpContentsForTests(), androidUnversionedBpContents: sdk.GetUnversionedAndroidBpContentsForTests(), androidVersionedBpContents: sdk.GetVersionedAndroidBpContentsForTests(), + snapshotTestCustomizations: map[snapshotTest]*snapshotTestCustomization{}, } buildParams := sdk.BuildParamsForTests() @@ -183,6 +184,24 @@ func getSdkSnapshotBuildInfo(t *testing.T, result *android.TestResult, sdk *sdk) return info } +// The enum of different sdk snapshot tests performed by CheckSnapshot. +type snapshotTest int + +const ( + // The enumeration of the different test configurations. + // A test with the snapshot/Android.bp file but without the original Android.bp file. + checkSnapshotWithoutSource snapshotTest = iota + + // A test with both the original source and the snapshot, with the source preferred. + checkSnapshotWithSourcePreferred + + // A test with both the original source and the snapshot, with the snapshot preferred. + checkSnapshotPreferredWithSource + + // The directory into which the snapshot will be 'unpacked'. + snapshotSubDir = "snapshot" +) + // Check the snapshot build rules. // // Takes a list of functions which check different facets of the snapshot build rules. @@ -214,31 +233,58 @@ func CheckSnapshot(t *testing.T, result *android.TestResult, name string, dir st // Populate a mock filesystem with the files that would have been copied by // the rules. fs := android.MockFS{} - snapshotSubDir := "snapshot" for _, dest := range snapshotBuildInfo.snapshotContents { fs[filepath.Join(snapshotSubDir, dest)] = nil } fs[filepath.Join(snapshotSubDir, "Android.bp")] = []byte(snapshotBuildInfo.androidBpContents) - preparer := result.Preparer() + // The preparers from the original source fixture. + sourcePreparers := result.Preparer() + + // Preparer to combine the snapshot and the source. + snapshotPreparer := android.GroupFixturePreparers(sourcePreparers, fs.AddToFixture()) + + var runSnapshotTestWithCheckers = func(t *testing.T, testConfig snapshotTest, extraPreparer android.FixturePreparer) { + customization := snapshotBuildInfo.snapshotTestCustomization(testConfig) + + // TODO(b/183184375): Set Config.TestAllowNonExistentPaths = false to verify that all the + // files the snapshot needs are actually copied into the snapshot. + + // Run the snapshot with the snapshot preparer and the extra preparer, which must come after as + // it may need to modify parts of the MockFS populated by the snapshot preparer. + result := android.GroupFixturePreparers(snapshotPreparer, extraPreparer). + ExtendWithErrorHandler(customization.errorHandler). + RunTest(t) + + // Perform any additional checks the test need on the result of processing the snapshot. + for _, checker := range customization.checkers { + checker(t, result) + } + } - // Process the generated bp file to make sure it is valid. Use the same preparer as was used to - // produce this result. t.Run("snapshot without source", func(t *testing.T) { - android.GroupFixturePreparers( - preparer, - // TODO(b/183184375): Set Config.TestAllowNonExistentPaths = false to verify that all the - // files the snapshot needs are actually copied into the snapshot. + // Remove the source Android.bp file to make sure it works without. + removeSourceAndroidBp := android.FixtureModifyMockFS(func(fs android.MockFS) { + delete(fs, "Android.bp") + }) - // Add the files (including bp) created for this snapshot to the test fixture. - fs.AddToFixture(), + runSnapshotTestWithCheckers(t, checkSnapshotWithoutSource, removeSourceAndroidBp) + }) - // Remove the source Android.bp file to make sure it works without. - // TODO(b/183184375): Add a test with the source. - android.FixtureModifyMockFS(func(fs android.MockFS) { - delete(fs, "Android.bp") - }), - ).RunTest(t) + t.Run("snapshot with source preferred", func(t *testing.T) { + runSnapshotTestWithCheckers(t, checkSnapshotWithSourcePreferred, android.NullFixturePreparer) + }) + + t.Run("snapshot preferred with source", func(t *testing.T) { + // Replace the snapshot/Android.bp file with one where "prefer: false," has been replaced with + // "prefer: true," + preferPrebuilts := android.FixtureModifyMockFS(func(fs android.MockFS) { + snapshotBpFile := filepath.Join(snapshotSubDir, "Android.bp") + unpreferred := string(fs[snapshotBpFile]) + fs[snapshotBpFile] = []byte(strings.ReplaceAll(unpreferred, "prefer: false,", "prefer: true,")) + }) + + runSnapshotTestWithCheckers(t, checkSnapshotPreferredWithSource, preferPrebuilts) }) } @@ -312,6 +358,46 @@ func checkMergeZips(expected ...string) snapshotBuildInfoChecker { } } +type resultChecker func(t *testing.T, result *android.TestResult) + +// snapshotTestChecker registers a checker that will be run against the result of processing the +// generated snapshot for the specified snapshotTest. +func snapshotTestChecker(snapshotTest snapshotTest, checker resultChecker) snapshotBuildInfoChecker { + return func(info *snapshotBuildInfo) { + customization := info.snapshotTestCustomization(snapshotTest) + customization.checkers = append(customization.checkers, checker) + } +} + +// snapshotTestErrorHandler registers an error handler to use when processing the snapshot +// in the specific test case. +// +// Generally, the snapshot should work with all the test cases but some do not and just in case +// there are a lot of issues to resolve, or it will take a lot of time this is a +// get-out-of-jail-free card that allows progress to be made. +// +// deprecated: should only be used as a temporary workaround with an attached to do and bug. +func snapshotTestErrorHandler(snapshotTest snapshotTest, handler android.FixtureErrorHandler) snapshotBuildInfoChecker { + return func(info *snapshotBuildInfo) { + customization := info.snapshotTestCustomization(snapshotTest) + customization.errorHandler = handler + } +} + +// Encapsulates information provided by each test to customize a specific snapshotTest. +type snapshotTestCustomization struct { + // Checkers that are run on the result of processing the preferred snapshot in a specific test + // case. + checkers []resultChecker + + // Specify an error handler for when processing a specific test case. + // + // In some cases the generated snapshot cannot be used in a test configuration. Those cases are + // invariably bugs that need to be resolved but sometimes that can take a while. This provides a + // mechanism to temporarily ignore that error. + errorHandler android.FixtureErrorHandler +} + // Encapsulates information about the snapshot build structure in order to insulate tests from // knowing too much about internal structures. // @@ -355,4 +441,21 @@ type snapshotBuildInfo struct { // The final output zip. outputZip string + + // The test specific customizations for each snapshot test. + snapshotTestCustomizations map[snapshotTest]*snapshotTestCustomization +} + +// snapshotTestCustomization gets the test specific customization for the specified snapshotTest. +// +// If no customization was created previously then it creates a default customization. +func (i *snapshotBuildInfo) snapshotTestCustomization(snapshotTest snapshotTest) *snapshotTestCustomization { + customization := i.snapshotTestCustomizations[snapshotTest] + if customization == nil { + customization = &snapshotTestCustomization{ + errorHandler: android.FixtureExpectsNoErrors, + } + i.snapshotTestCustomizations[snapshotTest] = customization + } + return customization }