Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d4e4b64e authored by Luca Farsi's avatar Luca Farsi
Browse files

Refactor package_outputs 

Refactor package_outputs in the TestOptimizer so it just returns a list
of soong_zip commands to be run by build_test_suites.

Since we already have a tested implementation for running subprocesses
in build_test_suites.py there's no reason to reimplement it in
optimized_targets.py. Because any packaging will ultimately use
soong_zip to package its final outputs change the code to just do
whatever prep it needs to and return a list of soong_zip commands.

This way the code is simpler to test without requiring subprocesses and
no reimplementation of subprocess running code is necessary.

Test: atest build_test_suites_test; atest optimized_targets_test
Bug: 358215235
Change-Id: I3025aefeeb7186f537266a72d8422211ca9835ba
parent 7c112657

ci/build_test_suites.py

+9 −8
Original line number Original line Diff line number Diff line
@@ -20,10 +20,8 @@ import json 
import logging

import logging

import os

import os

import pathlib

import pathlib

import re

import subprocess

import subprocess

import sys

import sys

from typing import Callable

from build_context import BuildContext

from build_context import BuildContext

import optimized_targets

import optimized_targets
@@ -70,7 +68,7 @@ class BuildPlanner: 
      return BuildPlan(set(self.args.extra_targets), set())

      return BuildPlan(set(self.args.extra_targets), set())





    build_targets = set()

    build_targets = set()

    packaging_functions = set()

    packaging_commands = []

    for target in self.args.extra_targets:

    for target in self.args.extra_targets:

      if self._unused_target_exclusion_enabled(

      if self._unused_target_exclusion_enabled(

          target

          target
@@ -86,9 +84,9 @@ class BuildPlanner: 
          target, self.build_context, self.args

          target, self.build_context, self.args

      )

      )

      build_targets.update(target_optimizer.get_build_targets())

      build_targets.update(target_optimizer.get_build_targets())

      packaging_functions.add(target_optimizer.package_outputs)

      packaging_commands.extend(target_optimizer.get_package_outputs_commands())





    return BuildPlan(build_targets, packaging_functions)

    return BuildPlan(build_targets, packaging_commands)





  def _unused_target_exclusion_enabled(self, target: str) -> bool:

  def _unused_target_exclusion_enabled(self, target: str) -> bool:

    return (

    return (
@@ -100,7 +98,7 @@ class BuildPlanner: 
@dataclass(frozen=True)

@dataclass(frozen=True)

class BuildPlan:

class BuildPlan:

  build_targets: set[str]

  build_targets: set[str]

  packaging_functions: set[Callable[..., None]]

  packaging_commands: list[list[str]]









def build_test_suites(argv: list[str]) -> int:

def build_test_suites(argv: list[str]) -> int:
@@ -182,8 +180,11 @@ def execute_build_plan(build_plan: BuildPlan): 
  except subprocess.CalledProcessError as e:

  except subprocess.CalledProcessError as e:

    raise BuildFailureError(e.returncode) from e

    raise BuildFailureError(e.returncode) from e





  for packaging_function in build_plan.packaging_functions:

  for packaging_command in build_plan.packaging_commands:

    packaging_function()

    try:

      run_command(packaging_command)

    except subprocess.CalledProcessError as e:

      raise BuildFailureError(e.returncode) from e









def get_top() -> pathlib.Path:

def get_top() -> pathlib.Path:

ci/build_test_suites_test.py

+16 −22
Original line number Original line Diff line number Diff line
@@ -241,17 +241,17 @@ class BuildPlannerTest(unittest.TestCase): 
  class TestOptimizedBuildTarget(optimized_targets.OptimizedBuildTarget):

  class TestOptimizedBuildTarget(optimized_targets.OptimizedBuildTarget):





    def __init__(

    def __init__(

        self, target, build_context, args, output_targets, packaging_outputs

        self, target, build_context, args, output_targets, packaging_commands

    ):

    ):

      super().__init__(target, build_context, args)

      super().__init__(target, build_context, args)

      self.output_targets = output_targets

      self.output_targets = output_targets

      self.packaging_outputs = packaging_outputs

      self.packaging_commands = packaging_commands





    def get_build_targets_impl(self):

    def get_build_targets_impl(self):

      return self.output_targets

      return self.output_targets





    def package_outputs_impl(self):

    def get_package_outputs_commands_impl(self):

      self.packaging_outputs.add(f'packaging {" ".join(self.output_targets)}')

      return self.packaging_commands





    def get_enabled_flag(self):

    def get_enabled_flag(self):

      return f'{self.target}_enabled'

      return f'{self.target}_enabled'
@@ -276,7 +276,7 @@ class BuildPlannerTest(unittest.TestCase): 




    build_plan = build_planner.create_build_plan()

    build_plan = build_planner.create_build_plan()





    self.assertEqual(len(build_plan.packaging_functions), 0)

    self.assertEqual(len(build_plan.packaging_commands), 0)





  def test_build_optimization_on_optimizes_target(self):

  def test_build_optimization_on_optimizes_target(self):

    build_targets = {'target_1', 'target_2'}

    build_targets = {'target_1', 'target_2'}
@@ -294,20 +294,19 @@ class BuildPlannerTest(unittest.TestCase): 




  def test_build_optimization_on_packages_target(self):

  def test_build_optimization_on_packages_target(self):

    build_targets = {'target_1', 'target_2'}

    build_targets = {'target_1', 'target_2'}

    packaging_outputs = set()

    optimized_target_name = self.get_optimized_target_name('target_1')

    packaging_commands = [[f'packaging {optimized_target_name}']]

    build_planner = self.create_build_planner(

    build_planner = self.create_build_planner(

        build_targets=build_targets,

        build_targets=build_targets,

        build_context=self.create_build_context(

        build_context=self.create_build_context(

            enabled_build_features=[{'name': self.get_target_flag('target_1')}]

            enabled_build_features=[{'name': self.get_target_flag('target_1')}]

        ),

        ),

        packaging_outputs=packaging_outputs,

        packaging_commands=packaging_commands,

    )

    )





    build_plan = build_planner.create_build_plan()

    build_plan = build_planner.create_build_plan()

    self.run_packaging_functions(build_plan)





    optimized_target_name = self.get_optimized_target_name('target_1')

    self.assertIn([f'packaging {optimized_target_name}'], build_plan.packaging_commands)

    self.assertIn(f'packaging {optimized_target_name}', packaging_outputs)





  def test_individual_build_optimization_off_doesnt_optimize(self):

  def test_individual_build_optimization_off_doesnt_optimize(self):

    build_targets = {'target_1', 'target_2'}

    build_targets = {'target_1', 'target_2'}
@@ -321,16 +320,15 @@ class BuildPlannerTest(unittest.TestCase): 




  def test_individual_build_optimization_off_doesnt_package(self):

  def test_individual_build_optimization_off_doesnt_package(self):

    build_targets = {'target_1', 'target_2'}

    build_targets = {'target_1', 'target_2'}

    packaging_outputs = set()

    packaging_commands = [['packaging command']]

    build_planner = self.create_build_planner(

    build_planner = self.create_build_planner(

        build_targets=build_targets,

        build_targets=build_targets,

        packaging_outputs=packaging_outputs,

        packaging_commands=packaging_commands,

    )

    )





    build_plan = build_planner.create_build_plan()

    build_plan = build_planner.create_build_plan()

    self.run_packaging_functions(build_plan)





    self.assertFalse(packaging_outputs)

    self.assertFalse(build_plan.packaging_commands)





  def test_target_output_used_target_built(self):

  def test_target_output_used_target_built(self):

    build_target = 'test_target'

    build_target = 'test_target'
@@ -408,7 +406,7 @@ class BuildPlannerTest(unittest.TestCase): 
      target_optimizations: dict[

      target_optimizations: dict[

          str, optimized_targets.OptimizedBuildTarget

          str, optimized_targets.OptimizedBuildTarget

      ] = None,

      ] = None,

      packaging_outputs: set[str] = set(),

      packaging_commands: list[list[str]] = [],

  ) -> build_test_suites.BuildPlanner:

  ) -> build_test_suites.BuildPlanner:

    if not build_context:

    if not build_context:

      build_context = self.create_build_context()

      build_context = self.create_build_context()
@@ -418,7 +416,7 @@ class BuildPlannerTest(unittest.TestCase): 
      target_optimizations = self.create_target_optimizations(

      target_optimizations = self.create_target_optimizations(

          build_context,

          build_context,

          build_targets,

          build_targets,

          packaging_outputs,

          packaging_commands,

      )

      )

    return build_test_suites.BuildPlanner(

    return build_test_suites.BuildPlanner(

        build_context, args, target_optimizations

        build_context, args, target_optimizations
@@ -450,14 +448,14 @@ class BuildPlannerTest(unittest.TestCase): 
      self,

      self,

      build_context: BuildContext,

      build_context: BuildContext,

      build_targets: set[str],

      build_targets: set[str],

      packaging_outputs: set[str] = set(),

      packaging_commands: list[list[str]] = [],

  ):

  ):

    target_optimizations = dict()

    target_optimizations = dict()

    for target in build_targets:

    for target in build_targets:

      target_optimizations[target] = functools.partial(

      target_optimizations[target] = functools.partial(

          self.TestOptimizedBuildTarget,

          self.TestOptimizedBuildTarget,

          output_targets={self.get_optimized_target_name(target)},

          output_targets={self.get_optimized_target_name(target)},

          packaging_outputs=packaging_outputs,

          packaging_commands=packaging_commands,

      )

      )





    return target_optimizations

    return target_optimizations
@@ -468,10 +466,6 @@ class BuildPlannerTest(unittest.TestCase): 
  def get_optimized_target_name(self, target: str):

  def get_optimized_target_name(self, target: str):

    return f'{target}_optimized'

    return f'{target}_optimized'





  def run_packaging_functions(self, build_plan: build_test_suites.BuildPlan):

    for packaging_function in build_plan.packaging_functions:

      packaging_function()



  def get_test_context(self, target: str):

  def get_test_context(self, target: str):

    return {

    return {

        'testInfos': [

        'testInfos': [

ci/optimized_targets.py

+10 −6
Original line number Original line Diff line number Diff line
@@ -52,14 +52,17 @@ class OptimizedBuildTarget(ABC): 
    self.modules_to_build = {self.target}

    self.modules_to_build = {self.target}

    return {self.target}

    return {self.target}





  def package_outputs(self):

  def get_package_outputs_commands(self) -> list[list[str]]:

    features = self.build_context.enabled_build_features

    features = self.build_context.enabled_build_features

    if self.get_enabled_flag() in features:

    if self.get_enabled_flag() in features:

      return self.package_outputs_impl()

      return self.get_package_outputs_commands_impl()





  def package_outputs_impl(self):

    return []



  def get_package_outputs_commands_impl(self) -> list[list[str]]:

    raise NotImplementedError(

    raise NotImplementedError(

        f'package_outputs_impl not implemented in {type(self).__name__}'

        'get_package_outputs_commands_impl not implemented in'

        f' {type(self).__name__}'

    )

    )





  def get_enabled_flag(self):

  def get_enabled_flag(self):
@@ -86,8 +89,8 @@ class NullOptimizer(OptimizedBuildTarget): 
  def get_build_targets(self):

  def get_build_targets(self):

    return {self.target}

    return {self.target}





  def package_outputs(self):

  def get_package_outputs_commands(self):

    pass

    return []









class ChangeInfo:

class ChangeInfo:
@@ -114,6 +117,7 @@ class ChangeInfo: 




    return changed_files

    return changed_files







class GeneralTestsOptimizer(OptimizedBuildTarget):

class GeneralTestsOptimizer(OptimizedBuildTarget):

  """general-tests optimizer

  """general-tests optimizer