From 567465da8cf04b436e82be5c0f2fa46f0f2001aa Mon Sep 17 00:00:00 2001 From: Paul Duffin Date: Tue, 16 Mar 2021 01:21:34 +0000 Subject: [PATCH] Add (String)Path(s)RelativeToTop and assert functions The existing NormalizePathForTesting function does not handle make install paths very well, as it returns a relative path with a leading "../" which is very confusing. It also does not clearly differentiate between the different paths. These functions return paths that are basically what are seen in a normal developer build, i.e. * * out/soong/ * out/ That makes tests that use them easier to understand. Follow up changes will clean up the existing usages of the Normalize... functions. Bug: 182885307 Test: m nothing Change-Id: I17ddc996bef5bbbf4a62da8334ea6ce29e306109 --- android/paths_test.go | 46 ++++++++++++++++++ android/test_asserts.go | 28 +++++++++++ android/testing.go | 105 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+) diff --git a/android/paths_test.go b/android/paths_test.go index 14a477353..3734ed201 100644 --- a/android/paths_test.go +++ b/android/paths_test.go @@ -1261,6 +1261,52 @@ func TestPathsForModuleSrc_AllowMissingDependencies(t *testing.T) { } } +func TestPathRelativeToTop(t *testing.T) { + testConfig := pathTestConfig("/tmp/build/top") + deviceTarget := Target{Os: Android, Arch: Arch{ArchType: Arm64}} + + ctx := &testModuleInstallPathContext{ + baseModuleContext: baseModuleContext{ + os: deviceTarget.Os, + target: deviceTarget, + }, + } + ctx.baseModuleContext.config = testConfig + + t.Run("install for soong", func(t *testing.T) { + p := PathForModuleInstall(ctx, "install/path") + AssertPathRelativeToTopEquals(t, "install path for soong", "out/soong/target/product/test_device/system/install/path", p) + }) + t.Run("install for make", func(t *testing.T) { + p := PathForModuleInstall(ctx, "install/path").ToMakePath() + AssertPathRelativeToTopEquals(t, "install path for make", "out/target/product/test_device/system/install/path", p) + }) + t.Run("output", func(t *testing.T) { + p := PathForOutput(ctx, "output/path") + AssertPathRelativeToTopEquals(t, "output path", "out/soong/output/path", p) + }) + t.Run("source", func(t *testing.T) { + p := PathForSource(ctx, "source/path") + AssertPathRelativeToTopEquals(t, "source path", "source/path", p) + }) + t.Run("mixture", func(t *testing.T) { + paths := Paths{ + PathForModuleInstall(ctx, "install/path"), + PathForModuleInstall(ctx, "install/path").ToMakePath(), + PathForOutput(ctx, "output/path"), + PathForSource(ctx, "source/path"), + } + + expected := []string{ + "out/soong/target/product/test_device/system/install/path", + "out/target/product/test_device/system/install/path", + "out/soong/output/path", + "source/path", + } + AssertPathsRelativeToTopEquals(t, "mixture", expected, paths) + }) +} + func ExampleOutputPath_ReplaceExtension() { ctx := &configErrorWrapper{ config: TestConfig("out", nil, "", nil), diff --git a/android/test_asserts.go b/android/test_asserts.go index d0de52393..5100abb79 100644 --- a/android/test_asserts.go +++ b/android/test_asserts.go @@ -40,6 +40,34 @@ func AssertStringEquals(t *testing.T, message string, expected string, actual st } } +// AssertPathRelativeToTopEquals checks if the expected value is equal to the result of calling +// PathRelativeToTop on the actual Path. +func AssertPathRelativeToTopEquals(t *testing.T, message string, expected string, actual Path) { + t.Helper() + AssertStringEquals(t, message, expected, PathRelativeToTop(actual)) +} + +// AssertPathsRelativeToTopEquals checks if the expected value is equal to the result of calling +// PathsRelativeToTop on the actual Paths. +func AssertPathsRelativeToTopEquals(t *testing.T, message string, expected []string, actual Paths) { + t.Helper() + AssertDeepEquals(t, message, expected, PathsRelativeToTop(actual)) +} + +// AssertStringPathRelativeToTopEquals checks if the expected value is equal to the result of calling +// StringPathRelativeToTop on the actual string path. +func AssertStringPathRelativeToTopEquals(t *testing.T, message string, config Config, expected string, actual string) { + t.Helper() + AssertStringEquals(t, message, expected, StringPathRelativeToTop(config.buildDir, actual)) +} + +// AssertStringPathsRelativeToTopEquals checks if the expected value is equal to the result of +// calling StringPathsRelativeToTop on the actual string paths. +func AssertStringPathsRelativeToTopEquals(t *testing.T, message string, config Config, expected []string, actual []string) { + t.Helper() + AssertDeepEquals(t, message, expected, StringPathsRelativeToTop(config.buildDir, actual)) +} + // AssertErrorMessageEquals checks if the error is not nil and has the expected message. If it does // not then this reports an error prefixed with the supplied message and including a reason for why // it failed. diff --git a/android/testing.go b/android/testing.go index dd3d607aa..af360fac9 100644 --- a/android/testing.go +++ b/android/testing.go @@ -813,6 +813,9 @@ func AndroidMkDataForTest(t *testing.T, ctx *TestContext, mod blueprint.Module) // that is relative to the root of the source tree. // // The build and source paths should be distinguishable based on their contents. +// +// deprecated: use PathRelativeToTop instead as it handles make install paths and differentiates +// between output and source properly. func NormalizePathForTesting(path Path) string { if path == nil { return "" @@ -828,6 +831,11 @@ func NormalizePathForTesting(path Path) string { return p } +// NormalizePathsForTesting creates a slice of strings where each string is the result of applying +// NormalizePathForTesting to the corresponding Path in the input slice. +// +// deprecated: use PathsRelativeToTop instead as it handles make install paths and differentiates +// between output and source properly. func NormalizePathsForTesting(paths Paths) []string { var result []string for _, path := range paths { @@ -836,3 +844,100 @@ func NormalizePathsForTesting(paths Paths) []string { } return result } + +// PathRelativeToTop returns a string representation of the path relative to a notional top +// directory. +// +// For a WritablePath it applies StringPathRelativeToTop to it, using the buildDir returned from the +// WritablePath's buildDir() method. For all other paths, i.e. source paths, that are already +// relative to the top it just returns their string representation. +func PathRelativeToTop(path Path) string { + if path == nil { + return "" + } + p := path.String() + if w, ok := path.(WritablePath); ok { + buildDir := w.buildDir() + return StringPathRelativeToTop(buildDir, p) + } + return p +} + +// PathsRelativeToTop creates a slice of strings where each string is the result of applying +// PathRelativeToTop to the corresponding Path in the input slice. +func PathsRelativeToTop(paths Paths) []string { + var result []string + for _, path := range paths { + relative := PathRelativeToTop(path) + result = append(result, relative) + } + return result +} + +// StringPathRelativeToTop returns a string representation of the path relative to a notional top +// directory. +// +// A standard build has the following structure: +// ../top/ +// out/ - make install files go here. +// out/soong - this is the buildDir passed to NewTestConfig() +// ... - the source files +// +// This function converts a path so that it appears relative to the ../top/ directory, i.e. +// * Make install paths, which have the pattern "buildDir/../" are converted into the top +// relative path "out/" +// * Soong install paths and other writable paths, which have the pattern "buildDir/" are +// converted into the top relative path "out/soong/". +// * Source paths are already relative to the top. +// +// This is provided for processing paths that have already been converted into a string, e.g. paths +// in AndroidMkEntries structures. As a result it needs to be supplied the soong output dir against +// which it can try and relativize paths. PathRelativeToTop must be used for process Path objects. +func StringPathRelativeToTop(soongOutDir string, path string) string { + + // A relative path must be a source path so leave it as it is. + if !filepath.IsAbs(path) { + return path + } + + // Check to see if the path is relative to the soong out dir. + rel, isRel, err := maybeRelErr(soongOutDir, path) + if err != nil { + panic(err) + } + + if isRel { + // The path is in the soong out dir so indicate that in the relative path. + return filepath.Join("out/soong", rel) + } + + // Check to see if the path is relative to the top level out dir. + outDir := filepath.Dir(soongOutDir) + rel, isRel, err = maybeRelErr(outDir, path) + if err != nil { + panic(err) + } + + if isRel { + // The path is in the out dir so indicate that in the relative path. + return filepath.Join("out", rel) + } + + // This should never happen. + panic(fmt.Errorf("internal error: absolute path %s is not relative to the out dir %s", path, outDir)) +} + +// StringPathsRelativeToTop creates a slice of strings where each string is the result of applying +// StringPathRelativeToTop to the corresponding string path in the input slice. +// +// This is provided for processing paths that have already been converted into a string, e.g. paths +// in AndroidMkEntries structures. As a result it needs to be supplied the soong output dir against +// which it can try and relativize paths. PathsRelativeToTop must be used for process Paths objects. +func StringPathsRelativeToTop(soongOutDir string, paths []string) []string { + var result []string + for _, path := range paths { + relative := StringPathRelativeToTop(soongOutDir, path) + result = append(result, relative) + } + return result +}