Add tests for converting license in androidmk

1. Added unit tests for license conversions in the androidmk tool, by
using a mocked filesystem;
2. Added integration tests in a shell script with a mocked filesystem.

Bug: 205615944
Test: m androidmk
Test: bash androidmk_test.sh
Test: TreeHugger
Change-Id: Id3d1f914190b52b82aadda7ea019192787b839d6

androidmk/androidmk/androidmk_test.go

@@ -1537,9 +1537,11 @@ android_app {
 	},
 	{
 		desc: "LOCAL_LICENSE_KINDS, LOCAL_LICENSE_CONDITIONS, LOCAL_NOTICE_FILE",
-		// TODO(b/205615944): When valid "android_license_files" exists, the test requires an Android.mk
+		// When "android_license_files" is valid, the test requires an Android.mk file
-		// file (and an Android.bp file is required as well if the license files locates outside the current
+		// outside the current (and an Android.bp file is required as well if the license
-		// directory). So plan to use a mock file system to mock the Android.mk and Android.bp files.
+		// files locates directory), thus a mock file system is needed. The integration
+		// test cases for these scenarios have been added in
+		// $(ANDROID_BUILD_TOP)/build/soong/tests/androidmk_test.sh.
 		in: `
 include $(CLEAR_VARS)
 LOCAL_MODULE := foo

bpfix/Android.bp

@@ -52,5 +52,6 @@ bootstrap_go_package {
     ],
     deps: [
         "blueprint-parser",
+        "blueprint-pathtools",
     ],
 }

bpfix/bpfix/bpfix.go

@@ -22,6 +22,7 @@ import (
 	"flag"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -29,6 +30,7 @@ import (
 	"strings"
 
 	"github.com/google/blueprint/parser"
+	"github.com/google/blueprint/pathtools"
 )
 
 // Reformat takes a blueprint file as a string and returns a formatted version
@@ -166,7 +168,7 @@ var fixStepsOnce = []FixStep{
 	},
 	{
 		Name: "rewriteLicenseProperties",
-		Fix:  runPatchListMod(rewriteLicenseProperties),
+		Fix:  runPatchListMod(rewriteLicenseProperty(nil, "")),
 	},
 }
 
@@ -1452,9 +1454,16 @@ func formatFlagProperties(mod *parser.Module, buf []byte, patchlist *parser.Patc
 	return nil
 }
 
+func rewriteLicenseProperty(fs pathtools.FileSystem, relativePath string) patchListModFunction {
+	return func(mod *parser.Module, buf []byte, patchList *parser.PatchList) error {
+		return rewriteLicenseProperties(mod, patchList, fs, relativePath)
+	}
+}
+
 // rewrite the "android_license_kinds" and "android_license_files" properties to a package module
 // (and a license module when needed).
-func rewriteLicenseProperties(mod *parser.Module, buf []byte, patchList *parser.PatchList) error {
+func rewriteLicenseProperties(mod *parser.Module, patchList *parser.PatchList, fs pathtools.FileSystem,
+	relativePath string) error {
 	// if a package module has been added, no more action is needed.
 	for _, patch := range *patchList {
 		if strings.Contains(patch.Replacement, "package {") {
@@ -1462,15 +1471,33 @@ func rewriteLicenseProperties(mod *parser.Module, buf []byte, patchList *parser.
 		}
 	}
 
+	// initial the fs
+	if fs == nil {
+		fs = pathtools.NewOsFs(os.Getenv("ANDROID_BUILD_TOP"))
+	}
+
+	// initial the relativePath
+	if len(relativePath) == 0 {
+		relativePath = getModuleRelativePath()
+	}
+	// validate the relativePath
+	ok := hasFile(relativePath+"/Android.mk", fs)
+	// some modules in the existing test cases in the androidmk_test.go do not have a valid path
+	if !ok && len(relativePath) > 0 {
+		return fmt.Errorf("Cannot find an Android.mk file at path %s", relativePath)
+	}
+
 	licenseKindsPropertyName := "android_license_kinds"
 	licenseFilesPropertyName := "android_license_files"
 
-	androidBpFileErr := "// Error: No Android.bp file is found at\n" +
+	androidBpFileErr := "// Error: No Android.bp file is found at path\n" +
 		"// %s\n" +
-		"// Please add one there with the needed license module first.\n"
+		"// Please add one there with the needed license module first.\n" +
+		"// Then reset the default_applicable_licenses property below with the license module name.\n"
 	licenseModuleErr := "// Error: Cannot get the name of the license module in the\n" +
 		"// %s file.\n" +
-		"// If no such license module exists, please add one there first.\n"
+		"// If no such license module exists, please add one there first.\n" +
+		"// Then reset the default_applicable_licenses property below with the license module name.\n"
 
 	defaultApplicableLicense := "Android-Apache-2.0"
 	var licenseModuleName, licensePatch string
@@ -1482,15 +1509,16 @@ func rewriteLicenseProperties(mod *parser.Module, buf []byte, patchList *parser.
 		// if have LOCAL_NOTICE_FILE outside the current directory, need to find and refer to the license
 		// module in the LOCAL_NOTICE_FILE location directly and no new license module needs to be created
 		if hasFileInParentDir {
-			bpPath, ok := getPathFromProperty(mod, licenseFilesPropertyName)
+			bpPath, ok := getPathFromProperty(mod, licenseFilesPropertyName, fs, relativePath)
 			if !ok {
-				bpDir, err := getDirFromProperty(mod, licenseFilesPropertyName)
+				bpDir, err := getDirFromProperty(mod, licenseFilesPropertyName, fs, relativePath)
 				if err != nil {
 					return err
 				}
 				licensePatch += fmt.Sprintf(androidBpFileErr, bpDir)
+				defaultApplicableLicense = ""
 			} else {
-				licenseModuleName, _ = getModuleName(bpPath, "license")
+				licenseModuleName, _ = getModuleName(bpPath, "license", fs)
 				if len(licenseModuleName) == 0 {
 					licensePatch += fmt.Sprintf(licenseModuleErr, bpPath)
 				}
@@ -1498,7 +1526,6 @@ func rewriteLicenseProperties(mod *parser.Module, buf []byte, patchList *parser.
 			}
 		} else {
 			// if have LOCAL_NOTICE_FILE in the current directory, need to create a new license module
-			relativePath := getModuleRelativePath()
 			if len(relativePath) == 0 {
 				return fmt.Errorf("Cannot obtain the relative path of the Android.mk file")
 			}
@@ -1617,17 +1644,14 @@ func getModuleAbsolutePath() string {
 	return absPath
 }
 
-// check whether a file exists in a directory
+// check whether a file exists in a filesystem
-func hasFile(dir string, fileName string) error {
+func hasFile(path string, fs pathtools.FileSystem) bool {
-	_, err := os.Stat(dir + fileName)
+	ok, _, _ := fs.Exists(path)
-	if err != nil {
+	return ok
-		return err
-	}
-	return nil
 }
 
 // get the directory where an `Android.bp` file and the property files are expected to locate
-func getDirFromProperty(mod *parser.Module, property string) (string, error) {
+func getDirFromProperty(mod *parser.Module, property string, fs pathtools.FileSystem, relativePath string) (string, error) {
 	listValue, ok := getLiteralListPropertyValue(mod, property)
 	if !ok {
 		// if do not find
@@ -1637,7 +1661,11 @@ func getDirFromProperty(mod *parser.Module, property string) (string, error) {
 		// if empty
 		return "", fmt.Errorf("Cannot find the value of the %s.%s property", mod.Type, property)
 	}
-	path := getModuleAbsolutePath()
+	_, isDir, _ := fs.Exists(relativePath)
+	if !isDir {
+		return "", fmt.Errorf("Cannot find the path %s", relativePath)
+	}
+	path := relativePath
 	for {
 		if !strings.HasPrefix(listValue[0], "../") {
 			break
@@ -1645,25 +1673,29 @@ func getDirFromProperty(mod *parser.Module, property string) (string, error) {
 		path = filepath.Dir(path)
 		listValue[0] = strings.TrimPrefix(listValue[0], "../")
 	}
+	_, isDir, _ = fs.Exists(path)
+	if !isDir {
+		return "", fmt.Errorf("Cannot find the path %s", path)
+	}
 	return path, nil
 }
 
 // get the path of the `Android.bp` file at the expected location where the property files locate
-func getPathFromProperty(mod *parser.Module, property string) (string, bool) {
+func getPathFromProperty(mod *parser.Module, property string, fs pathtools.FileSystem, relativePath string) (string, bool) {
-	dir, err := getDirFromProperty(mod, property)
+	dir, err := getDirFromProperty(mod, property, fs, relativePath)
 	if err != nil {
 		return "", false
 	}
-	err = hasFile(dir, "/Android.bp")
+	ok := hasFile(dir+"/Android.bp", fs)
-	if err != nil {
+	if !ok {
 		return "", false
 	}
 	return dir + "/Android.bp", true
 }
 
 // parse an Android.bp file to get the name of the first module with type of moduleType
-func getModuleName(path string, moduleType string) (string, error) {
+func getModuleName(path string, moduleType string, fs pathtools.FileSystem) (string, error) {
-	tree, err := parserPath(path)
+	tree, err := parserPath(path, fs)
 	if err != nil {
 		return "", err
 	}
@@ -1685,8 +1717,13 @@ func getModuleName(path string, moduleType string) (string, error) {
 }
 
 // parse an Android.bp file with the specific path
-func parserPath(path string) (tree *parser.File, err error) {
+func parserPath(path string, fs pathtools.FileSystem) (tree *parser.File, err error) {
-	fileContent, _ := os.ReadFile(path)
+	f, err := fs.Open(path)
+	if err != nil {
+		return tree, err
+	}
+	defer f.Close()
+	fileContent, _ := ioutil.ReadAll(f)
 	tree, err = parse(path, bytes.NewBufferString(string(fileContent)))
 	if err != nil {
 		return tree, err

bpfix/bpfix/bpfix_test.go

@@ -25,6 +25,7 @@ import (
 	"reflect"
 
 	"github.com/google/blueprint/parser"
+	"github.com/google/blueprint/pathtools"
 )
 
 // TODO(jeffrygaston) remove this when position is removed from ParseNode (in b/38325146) and we can directly do reflect.DeepEqual
@@ -1678,10 +1679,20 @@ func TestFormatFlagProperty(t *testing.T) {
 	}
 }
 
-func TestRewriteLicenseProperties(t *testing.T) {
+func TestRewriteLicenseProperty(t *testing.T) {
+	mockFs := pathtools.MockFs(map[string][]byte{
+		"a/b/c/d/Android.mk": []byte("this is not important."),
+		"a/b/LicenseFile1":   []byte("LicenseFile1"),
+		"a/b/LicenseFile2":   []byte("LicenseFile2"),
+		"a/b/Android.bp":     []byte("license {\n\tname: \"reuse_a_b_license\",\n}\n"),
+	})
+	relativePath := "a/b/c/d"
+	relativePathErr := "a/b/c"
 	tests := []struct {
 		name string
 		in   string
+		fs   pathtools.FileSystem
+		path string
 		out  string
 	}{
 		{
@@ -1744,13 +1755,194 @@ func TestRewriteLicenseProperties(t *testing.T) {
 				}
 			`,
 		},
-		// TODO(b/205615944): When valid "android_license_files" exists, the test requires an Android.mk
+		{
-		// file (and an Android.bp file is required as well if the license files locates outside the current
+			name: "license rewriting with license files in the current directory",
-		// directory). So plan to use a mock file system to mock the Android.mk and Android.bp files.
+			in: `
+				android_test {
+					name: "foo",
+					android_license_kinds: ["license_kind"],
+					android_license_conditions: ["license_notice"],
+					android_license_files: ["LicenseFile1", "LicenseFile2",],
+				}
+			`,
+			fs:   mockFs,
+			path: relativePath,
+			out: `
+			package {
+				// See: http://go/android-license-faq
+				default_applicable_licenses: [
+					"a_b_c_d_license",
+				],
+			}
+
+			license {
+				name: "a_b_c_d_license",
+				visibility: [":__subpackages__"],
+				license_kinds: [
+					"license_kind",
+				],
+				license_text: [
+					"LicenseFile1",
+					"LicenseFile2",
+				],
+			}
+
+			android_test {
+				name: "foo",
+				android_license_kinds: ["license_kind"],
+				android_license_conditions: ["license_notice"],
+				android_license_files: [
+					"LicenseFile1",
+					"LicenseFile2",
+				],
+			}
+			`,
+		},
+		{
+			name: "license rewriting with license files outside the current directory",
+			in: `
+				android_test {
+					name: "foo",
+					android_license_kinds: ["license_kind"],
+					android_license_conditions: ["license_notice"],
+					android_license_files: ["../../LicenseFile1", "../../LicenseFile2",],
+				}
+			`,
+			fs:   mockFs,
+			path: relativePath,
+			out: `
+			package {
+				// See: http://go/android-license-faq
+				default_applicable_licenses: [
+					"reuse_a_b_license",
+				],
+			}
+
+			android_test {
+				name: "foo",
+				android_license_kinds: ["license_kind"],
+				android_license_conditions: ["license_notice"],
+				android_license_files: [
+					"../../LicenseFile1",
+					"../../LicenseFile2",
+				],
+			}
+			`,
+		},
+		{
+			name: "license rewriting with no Android.bp file in the expected location",
+			in: `
+				android_test {
+					name: "foo",
+					android_license_kinds: ["license_kind"],
+					android_license_conditions: ["license_notice"],
+					android_license_files: ["../../LicenseFile1", "../../LicenseFile2",],
+				}
+			`,
+			fs: pathtools.MockFs(map[string][]byte{
+				"a/b/c/d/Android.mk": []byte("this is not important."),
+				"a/b/LicenseFile1":   []byte("LicenseFile1"),
+				"a/b/LicenseFile2":   []byte("LicenseFile2"),
+				"a/Android.bp":       []byte("license {\n\tname: \"reuse_a_b_license\",\n}\n"),
+			}),
+			path: relativePath,
+			out: `
+			// Error: No Android.bp file is found at path
+			// a/b
+			// Please add one there with the needed license module first.
+			// Then reset the default_applicable_licenses property below with the license module name.
+			package {
+				// See: http://go/android-license-faq
+				default_applicable_licenses: [
+					"",
+				],
+			}
+
+			android_test {
+				name: "foo",
+				android_license_kinds: ["license_kind"],
+				android_license_conditions: ["license_notice"],
+				android_license_files: [
+					"../../LicenseFile1",
+					"../../LicenseFile2",
+				],
+			}
+			`,
+		},
+		{
+			name: "license rewriting with an Android.bp file without a license module",
+			in: `
+				android_test {
+					name: "foo",
+					android_license_kinds: ["license_kind"],
+					android_license_conditions: ["license_notice"],
+					android_license_files: ["../../LicenseFile1", "../../LicenseFile2",],
+				}
+			`,
+			fs: pathtools.MockFs(map[string][]byte{
+				"a/b/c/d/Android.mk": []byte("this is not important."),
+				"a/b/LicenseFile1":   []byte("LicenseFile1"),
+				"a/b/LicenseFile2":   []byte("LicenseFile2"),
+				"a/b/Android.bp":     []byte("non_license {\n\tname: \"reuse_a_b_license\",\n}\n"),
+			}),
+			path: relativePath,
+			out: `
+			// Error: Cannot get the name of the license module in the
+			// a/b/Android.bp file.
+			// If no such license module exists, please add one there first.
+			// Then reset the default_applicable_licenses property below with the license module name.
+			package {
+				// See: http://go/android-license-faq
+				default_applicable_licenses: [
+					"",
+				],
+			}
+
+			android_test {
+				name: "foo",
+				android_license_kinds: ["license_kind"],
+				android_license_conditions: ["license_notice"],
+				android_license_files: [
+					"../../LicenseFile1",
+					"../../LicenseFile2",
+				],
+			}
+			`,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			runPassOnce(t, test.in, test.out, runPatchListMod(rewriteLicenseProperties))
+			runPassOnce(t, test.in, test.out, runPatchListMod(rewriteLicenseProperty(test.fs, test.path)))
+		})
+	}
+
+	testErrs := []struct {
+		name        string
+		in          string
+		fs          pathtools.FileSystem
+		path        string
+		expectedErr string
+	}{
+		{
+			name: "license rewriting with a wrong path",
+			in: `
+				android_test {
+					name: "foo",
+					android_license_kinds: ["license_kind"],
+					android_license_conditions: ["license_notice"],
+					android_license_files: ["../../LicenseFile1", "../../LicenseFile2",],
+				}
+			`,
+			fs:   mockFs,
+			path: relativePathErr,
+			expectedErr: `
+				Cannot find an Android.mk file at path a/b/c
+			`,
+		},
+	}
+	for _, test := range testErrs {
+		t.Run(test.name, func(t *testing.T) {
+			checkError(t, test.in, test.expectedErr, runPatchListMod(rewriteLicenseProperty(test.fs, test.path)))
 		})
 	}
 }

tests/androidmk_test.sh

@@ -0,0 +1,135 @@
+#!/bin/bash -eu
+
+set -o pipefail
+
+# How to run: bash path-to-script/androidmk_test.sh
+# Tests of converting license functionality of the androidmk tool
+REAL_TOP="$(readlink -f "$(dirname "$0")"/../../..)"
+$REAL_TOP/build/soong/soong_ui.bash --make-mode androidmk
+
+source "$(dirname "$0")/lib.sh"
+
+# Expect to create a new license module
+function test_rewrite_license_property_inside_current_directory {
+  setup
+
+  # Create an Android.mk file
+  mkdir -p a/b
+  cat > a/b/Android.mk <<'EOF'
+include $(CLEAR_VARS)
+LOCAL_MODULE := foo
+LOCAL_LICENSE_KINDS := license_kind1 license_kind2
+LOCAL_LICENSE_CONDITIONS := license_condition
+LOCAL_NOTICE_FILE := $(LOCAL_PATH)/license_notice1 $(LOCAL_PATH)/license_notice2
+include $(BUILD_PACKAGE)
+EOF
+
+  # Create an expected Android.bp file for the module "foo"
+  cat > a/b/Android.bp <<'EOF'
+package {
+    // See: http://go/android-license-faq
+    default_applicable_licenses: [
+        "a_b_license",
+    ],
+}
+
+license {
+    name: "a_b_license",
+    visibility: [":__subpackages__"],
+    license_kinds: [
+        "license_kind1",
+        "license_kind2",
+    ],
+    license_text: [
+        "license_notice1",
+        "license_notice2",
+    ],
+}
+
+android_app {
+    name: "foo",
+}
+EOF
+
+  run_androidmk_test "a/b/Android.mk" "a/b/Android.bp"
+}
+
+# Expect to reference to an existing license module
+function test_rewrite_license_property_outside_current_directory {
+  setup
+
+  # Create an Android.mk file
+  mkdir -p a/b/c/d
+  cat > a/b/c/d/Android.mk <<'EOF'
+include $(CLEAR_VARS)
+LOCAL_MODULE := foo
+LOCAL_LICENSE_KINDS := license_kind1 license_kind2
+LOCAL_LICENSE_CONDITIONS := license_condition
+LOCAL_NOTICE_FILE := $(LOCAL_PATH)/../../license_notice1 $(LOCAL_PATH)/../../license_notice2
+include $(BUILD_PACKAGE)
+EOF
+
+  # Create an expected (input) Android.bp file at a/b/
+  cat > a/b/Android.bp <<'EOF'
+package {
+    // See: http://go/android-license-faq
+    default_applicable_licenses: [
+        "a_b_license",
+    ],
+}
+
+license {
+    name: "a_b_license",
+    visibility: [":__subpackages__"],
+    license_kinds: [
+        "license_kind1",
+        "license_kind2",
+    ],
+    license_text: [
+        "license_notice1",
+        "license_notice2",
+    ],
+}
+
+android_app {
+    name: "bar",
+}
+EOF
+
+  # Create an expected (output) Android.bp file for the module "foo"
+  cat > a/b/c/d/Android.bp <<'EOF'
+package {
+    // See: http://go/android-license-faq
+    default_applicable_licenses: [
+        "a_b_license",
+    ],
+}
+
+android_app {
+    name: "foo",
+}
+EOF
+
+  run_androidmk_test "a/b/c/d/Android.mk" "a/b/c/d/Android.bp"
+}
+
+run_androidmk_test () {
+  export ANDROID_BUILD_TOP="$MOCK_TOP"
+
+  local out=$($REAL_TOP/*/host/*/bin/androidmk "$1")
+  local expected=$(<"$2")
+
+  if [[ "$out" != "$expected" ]]; then
+    ANDROID_BUILD_TOP="$REAL_TOP"
+    cleanup_mock_top
+    fail "The output is not the same as the expected"
+  fi
+
+  ANDROID_BUILD_TOP="$REAL_TOP"
+  cleanup_mock_top
+  echo "Succeeded"
+}
+
+test_rewrite_license_property_inside_current_directory
+
+test_rewrite_license_property_outside_current_directory

tests/run_integration_tests.sh

@@ -3,6 +3,7 @@
 set -o pipefail
 
 TOP="$(readlink -f "$(dirname "$0")"/../../..)"
+"$TOP/build/soong/tests/androidmk_test.sh"
 "$TOP/build/soong/tests/bootstrap_test.sh"
 "$TOP/build/soong/tests/mixed_mode_test.sh"
 "$TOP/build/soong/tests/bp2build_bazel_test.sh"