Merge "Save the target file when applypatch tests fail"
diff --git a/recovery.cpp b/recovery.cpp
index 7c83744..d3af357 100644
--- a/recovery.cpp
+++ b/recovery.cpp
@@ -116,12 +116,6 @@
 // We will try to apply the update package 5 times at most in case of an I/O error or
 // bspatch | imgpatch error.
 static const int RETRY_LIMIT = 4;
-static const int BATTERY_READ_TIMEOUT_IN_SEC = 10;
-// GmsCore enters recovery mode to install package when having enough battery
-// percentage. Normally, the threshold is 40% without charger and 20% with charger.
-// So we should check battery with a slightly lower limitation.
-static const int BATTERY_OK_PERCENTAGE = 20;
-static const int BATTERY_WITH_CHARGER_OK_PERCENTAGE = 15;
 static constexpr const char* RECOVERY_WIPE = "/etc/recovery.wipe";
 static constexpr const char* DEFAULT_LOCALE = "en-US";
 
@@ -1265,58 +1259,65 @@
   }
 }
 
-static bool is_battery_ok() {
-    struct healthd_config healthd_config = {
-            .batteryStatusPath = android::String8(android::String8::kEmptyString),
-            .batteryHealthPath = android::String8(android::String8::kEmptyString),
-            .batteryPresentPath = android::String8(android::String8::kEmptyString),
-            .batteryCapacityPath = android::String8(android::String8::kEmptyString),
-            .batteryVoltagePath = android::String8(android::String8::kEmptyString),
-            .batteryTemperaturePath = android::String8(android::String8::kEmptyString),
-            .batteryTechnologyPath = android::String8(android::String8::kEmptyString),
-            .batteryCurrentNowPath = android::String8(android::String8::kEmptyString),
-            .batteryCurrentAvgPath = android::String8(android::String8::kEmptyString),
-            .batteryChargeCounterPath = android::String8(android::String8::kEmptyString),
-            .batteryFullChargePath = android::String8(android::String8::kEmptyString),
-            .batteryCycleCountPath = android::String8(android::String8::kEmptyString),
-            .energyCounter = NULL,
-            .boot_min_cap = 0,
-            .screen_on = NULL
-    };
-    healthd_board_init(&healthd_config);
+static bool is_battery_ok(int* required_battery_level) {
+  struct healthd_config healthd_config = {
+    .batteryStatusPath = android::String8(android::String8::kEmptyString),
+    .batteryHealthPath = android::String8(android::String8::kEmptyString),
+    .batteryPresentPath = android::String8(android::String8::kEmptyString),
+    .batteryCapacityPath = android::String8(android::String8::kEmptyString),
+    .batteryVoltagePath = android::String8(android::String8::kEmptyString),
+    .batteryTemperaturePath = android::String8(android::String8::kEmptyString),
+    .batteryTechnologyPath = android::String8(android::String8::kEmptyString),
+    .batteryCurrentNowPath = android::String8(android::String8::kEmptyString),
+    .batteryCurrentAvgPath = android::String8(android::String8::kEmptyString),
+    .batteryChargeCounterPath = android::String8(android::String8::kEmptyString),
+    .batteryFullChargePath = android::String8(android::String8::kEmptyString),
+    .batteryCycleCountPath = android::String8(android::String8::kEmptyString),
+    .energyCounter = nullptr,
+    .boot_min_cap = 0,
+    .screen_on = nullptr
+  };
+  healthd_board_init(&healthd_config);
 
-    android::BatteryMonitor monitor;
-    monitor.init(&healthd_config);
+  android::BatteryMonitor monitor;
+  monitor.init(&healthd_config);
 
-    int wait_second = 0;
-    while (true) {
-        int charge_status = monitor.getChargeStatus();
-        // Treat unknown status as charged.
-        bool charged = (charge_status != android::BATTERY_STATUS_DISCHARGING &&
-                        charge_status != android::BATTERY_STATUS_NOT_CHARGING);
-        android::BatteryProperty capacity;
-        android::status_t status = monitor.getProperty(android::BATTERY_PROP_CAPACITY, &capacity);
-        ui_print("charge_status %d, charged %d, status %d, capacity %" PRId64 "\n", charge_status,
-                 charged, status, capacity.valueInt64);
-        // At startup, the battery drivers in devices like N5X/N6P take some time to load
-        // the battery profile. Before the load finishes, it reports value 50 as a fake
-        // capacity. BATTERY_READ_TIMEOUT_IN_SEC is set that the battery drivers are expected
-        // to finish loading the battery profile earlier than 10 seconds after kernel startup.
-        if (status == 0 && capacity.valueInt64 == 50) {
-            if (wait_second < BATTERY_READ_TIMEOUT_IN_SEC) {
-                sleep(1);
-                wait_second++;
-                continue;
-            }
-        }
-        // If we can't read battery percentage, it may be a device without battery. In this
-        // situation, use 100 as a fake battery percentage.
-        if (status != 0) {
-            capacity.valueInt64 = 100;
-        }
-        return (charged && capacity.valueInt64 >= BATTERY_WITH_CHARGER_OK_PERCENTAGE) ||
-                (!charged && capacity.valueInt64 >= BATTERY_OK_PERCENTAGE);
+  static constexpr int BATTERY_READ_TIMEOUT_IN_SEC = 10;
+  int wait_second = 0;
+  while (true) {
+    int charge_status = monitor.getChargeStatus();
+    // Treat unknown status as charged.
+    bool charged = (charge_status != android::BATTERY_STATUS_DISCHARGING &&
+                    charge_status != android::BATTERY_STATUS_NOT_CHARGING);
+    android::BatteryProperty capacity;
+    android::status_t status = monitor.getProperty(android::BATTERY_PROP_CAPACITY, &capacity);
+    ui_print("charge_status %d, charged %d, status %d, capacity %" PRId64 "\n", charge_status,
+             charged, status, capacity.valueInt64);
+    // At startup, the battery drivers in devices like N5X/N6P take some time to load
+    // the battery profile. Before the load finishes, it reports value 50 as a fake
+    // capacity. BATTERY_READ_TIMEOUT_IN_SEC is set that the battery drivers are expected
+    // to finish loading the battery profile earlier than 10 seconds after kernel startup.
+    if (status == 0 && capacity.valueInt64 == 50) {
+      if (wait_second < BATTERY_READ_TIMEOUT_IN_SEC) {
+        sleep(1);
+        wait_second++;
+        continue;
+      }
     }
+    // If we can't read battery percentage, it may be a device without battery. In this situation,
+    // use 100 as a fake battery percentage.
+    if (status != 0) {
+      capacity.valueInt64 = 100;
+    }
+
+    // GmsCore enters recovery mode to install package when having enough battery percentage.
+    // Normally, the threshold is 40% without charger and 20% with charger. So we should check
+    // battery with a slightly lower limitation.
+    static constexpr int BATTERY_OK_PERCENTAGE = 20;
+    static constexpr int BATTERY_WITH_CHARGER_OK_PERCENTAGE = 15;
+    *required_battery_level = charged ? BATTERY_WITH_CHARGER_OK_PERCENTAGE : BATTERY_OK_PERCENTAGE;
+    return capacity.valueInt64 >= *required_battery_level;
+  }
 }
 
 // Set the retry count to |retry_count| in BCB.
@@ -1544,9 +1545,10 @@
     // to log the update attempt since update_package is non-NULL.
     modified_flash = true;
 
-    if (retry_count == 0 && !is_battery_ok()) {
-      ui->Print("battery capacity is not enough for installing package, needed is %d%%\n",
-                BATTERY_OK_PERCENTAGE);
+    int required_battery_level;
+    if (retry_count == 0 && !is_battery_ok(&required_battery_level)) {
+      ui->Print("battery capacity is not enough for installing package: %d%% needed\n",
+                required_battery_level);
       // Log the error code to last_install when installation skips due to
       // low battery.
       log_failure_code(kLowBattery, update_package);
diff --git a/updater_sample/tests/res/raw/ota_002_package.zip b/updater_sample/tests/res/raw/ota_002_package.zip
index 3bfe377..145c62e 100644
--- a/updater_sample/tests/res/raw/ota_002_package.zip
+++ b/updater_sample/tests/res/raw/ota_002_package.zip
Binary files differ
diff --git a/updater_sample/tests/res/raw/update_config_stream_002.json b/updater_sample/tests/res/raw/update_config_stream_002.json
index 36c5722..f00f19c 100644
--- a/updater_sample/tests/res/raw/update_config_stream_002.json
+++ b/updater_sample/tests/res/raw/update_config_stream_002.json
@@ -1,35 +1,35 @@
 {
     "__": "*** Generated using tools/gen_update_config.py ***",
-    "name": "S ota_002",
-    "streaming_metadata": {
+    "ab_install_type": "STREAMING",
+    "ab_streaming_metadata": {
         "property_files": [
             {
                 "filename": "payload.bin",
-                "offset": 195,
-                "size": 8
+                "offset": 41,
+                "size": 7
             },
             {
                 "filename": "payload_properties.txt",
-                "offset": 276,
-                "size": 19
+                "offset": 100,
+                "size": 18
             },
             {
                 "filename": "care_map.txt",
-                "offset": 42,
-                "size": 9
+                "offset": 160,
+                "size": 8
             },
             {
                 "filename": "compatibility.zip",
-                "offset": 119,
-                "size": 14
+                "offset": 215,
+                "size": 13
             },
             {
                 "filename": "metadata",
-                "offset": 375,
-                "size": 9
+                "offset": 287,
+                "size": 8
             }
         ]
     },
-    "type": "STREAMING",
+    "name": "S ota_002_package",
     "url": "file:///data/sample-ota-packages/ota_002_package.zip"
-}
+}
\ No newline at end of file
diff --git a/updater_sample/tools/gen_update_config.py b/updater_sample/tools/gen_update_config.py
index 44e9ac3..cb9bd01 100755
--- a/updater_sample/tools/gen_update_config.py
+++ b/updater_sample/tools/gen_update_config.py
@@ -31,7 +31,7 @@
 import zipfile
 
 
-class GenUpdateConfig(object): # pylint: disable=too-few-public-methods
+class GenUpdateConfig(object):
     """
     A class that generates update configuration file from an OTA package.
 
@@ -43,9 +43,8 @@
     AB_INSTALL_TYPE_NON_STREAMING = 'NON_STREAMING'
     METADATA_NAME = 'META-INF/com/android/metadata'
 
-    def __init__(self, package, out, url, ab_install_type):
+    def __init__(self, package, url, ab_install_type):
         self.package = package
-        self.out = out
         self.url = url
         self.ab_install_type = ab_install_type
         self.streaming_required = (
@@ -59,14 +58,20 @@
             # compatibility.zip is available only if target supports Treble.
             'compatibility.zip',
         )
+        self._config = None
+
+    @property
+    def config(self):
+        """Returns generated config object."""
+        return self._config
 
     def run(self):
-        """generate config"""
+        """Generates config."""
         streaming_metadata = None
         if self.ab_install_type == GenUpdateConfig.AB_INSTALL_TYPE_STREAMING:
             streaming_metadata = self._gen_ab_streaming_metadata()
 
-        config = {
+        self._config = {
             '__': '*** Generated using tools/gen_update_config.py ***',
             'name': self.ab_install_type[0] + ' ' + os.path.basename(self.package)[:-4],
             'url': self.url,
@@ -74,12 +79,8 @@
             'ab_install_type': self.ab_install_type,
         }
 
-        with open(self.out, 'w') as out:
-            json.dump(config, out, indent=4, separators=(',', ': '), sort_keys=True)
-            print('Config is written to ' + out.name)
-
     def _gen_ab_streaming_metadata(self):
-        """Open zip file and get metadata for files required for streaming update."""
+        """Builds metadata for files required for streaming update."""
         with zipfile.ZipFile(self.package, 'r') as package_zip:
             property_files = self._get_property_files(package_zip)
 
@@ -90,7 +91,7 @@
         return metadata
 
     def _get_property_files(self, zip_file):
-        """Constructs the property-files list for A/B streaming metadata"""
+        """Constructs the property-files list for A/B streaming metadata."""
 
         def compute_entry_offset_size(name):
             """Computes the zip entry offset and size."""
@@ -115,8 +116,13 @@
 
         return property_files
 
+    def write(self, out):
+        """Writes config to the output file."""
+        with open(out, 'w') as out_file:
+            json.dump(self.config, out_file, indent=4, separators=(',', ': '), sort_keys=True)
 
-def main(): # pylint: disable=missing-docstring
+
+def main():  # pylint: disable=missing-docstring
     ab_install_type_choices = [
         GenUpdateConfig.AB_INSTALL_TYPE_STREAMING,
         GenUpdateConfig.AB_INSTALL_TYPE_NON_STREAMING]
@@ -144,10 +150,11 @@
 
     gen = GenUpdateConfig(
         package=args.package,
-        out=args.out,
         url=args.url,
         ab_install_type=args.ab_install_type)
     gen.run()
+    gen.write(args.out)
+    print('Config is written to ' + args.out)
 
 
 if __name__ == '__main__':
diff --git a/updater_sample/tools/gen_update_config_test.py b/updater_sample/tools/gen_update_config_test.py
new file mode 100755
index 0000000..951d4c4
--- /dev/null
+++ b/updater_sample/tools/gen_update_config_test.py
@@ -0,0 +1,55 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2018 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""
+Tests gen_update_config.py
+"""
+
+import os.path
+import unittest
+from gen_update_config import GenUpdateConfig
+
+
+class GenUpdateConfigTest(unittest.TestCase): # pylint: disable=missing-docstring
+
+    def test_ab_install_type_streaming(self):
+        """tests if streaming property files' offset and size are generated properly"""
+        config, package = self._generate_config()
+        property_files = config['ab_streaming_metadata']['property_files']
+        self.assertEqual(len(property_files), 5)
+        with open(package, 'rb') as pkg_file:
+            for prop in property_files:
+                filename, offset, size = prop['filename'], prop['offset'], prop['size']
+                pkg_file.seek(offset)
+                data = pkg_file.read(size).decode('ascii')
+                # data in the archive are just uppercase filenames without extension
+                expected_data = filename.split('.')[0].upper()
+                self.assertEqual(data, expected_data)
+
+    @staticmethod
+    def _generate_config():
+        """Generates JSON config from ota_002_package.zip."""
+        ota_package = os.path.join(os.path.dirname(__file__),
+                                   '../tests/res/raw/ota_002_package.zip')
+        gen = GenUpdateConfig(ota_package,
+                              'file:///foo.bar',
+                              GenUpdateConfig.AB_INSTALL_TYPE_STREAMING)
+        gen.run()
+        return gen.config, ota_package
+
+
+if __name__ == '__main__':
+    unittest.main()