Skip to content

Commit

Permalink
Merge pull request #1381 from buildtesters/buildtest_build_limit
Browse files Browse the repository at this point in the history
add support for limiting builds via 'buildtest build --limit'
  • Loading branch information
shahzebsiddiqui authored Feb 13, 2023
2 parents 65ce2a9 + ebae3c0 commit 4c4b42b
Show file tree
Hide file tree
Showing 62 changed files with 122 additions and 194 deletions.
2 changes: 1 addition & 1 deletion bash_completion.sh
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ _buildtest ()
#case "${prev}" in
build|bd)
local shortoption="-b -e -et -f -m -s -t -u -x"
local longoption="--buildspec --disable-executor-check --executor --executor-type --exclude --filter --helpfilter --maxpendtime --modules --module-purge --nodes --pollinterval --procs --rerun --remove-stagedir --retry --stage --tags --timeout --unload-modules"
local longoption="--buildspec --disable-executor-check --executor --executor-type --exclude --filter --helpfilter --limit --maxpendtime --modules --module-purge --nodes --pollinterval --procs --rerun --remove-stagedir --retry --stage --tags --timeout --unload-modules"
local allopts="${longoption} ${shortoption}"

COMPREPLY=( $( compgen -W "$allopts" -- $cur ) )
Expand Down
6 changes: 0 additions & 6 deletions buildtest/builders/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,7 +546,6 @@ def _write_build_script(self, modules=None, modulepurge=None, unload_modules=Non
if modules:
lines.append("# Specify list of modules to load")
for module in modules.split(","):

lines.append(f"module load {module}")

lines += [
Expand Down Expand Up @@ -716,13 +715,11 @@ def get_job_directives(self):
lines += [f"#BSUB -e {self.name}.err"]

if self.pbs:

for line in self.pbs:
lines.append(f"#PBS {line}")
lines.append(f"#PBS -N {self.name}")

if self.cobalt:

for line in self.cobalt:
lines.append(f"#COBALT {line}")
lines.append(f"#COBALT --jobname={self.name}")
Expand Down Expand Up @@ -868,13 +865,11 @@ def add_metrics(self):
return

for key in self.metrics.keys():

# default value of metric is empty string
self.metadata["metrics"][key] = ""

# apply regex on stdout/stderr and assign value to metrics
if self.metrics[key].get("regex"):

if self.metrics[key]["regex"]["stream"] == "stdout":
content = self._output
elif self.metrics[key]["regex"]["stream"] == "stderr":
Expand Down Expand Up @@ -962,7 +957,6 @@ def check_test_state(self):

# if status is defined in Buildspec, then check for returncode and regex
if self.status:

slurm_job_state_match = False
pbs_job_state_match = False
lsf_job_state_match = False
Expand Down
2 changes: 0 additions & 2 deletions buildtest/builders/compiler.py
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,6 @@ def _process_compiler_config(self):

# if default compiler setting provided in buildspec let's assign it.
if deep_get(self.compiler_section, "default", self.compiler_group):

self.cc = (
self.compiler_section["default"][self.compiler_group].get("cc")
or self.cc
Expand Down Expand Up @@ -462,7 +461,6 @@ def _process_compiler_config(self):
)
# if compiler instance defined in config section read from buildspec. This overrides default section if specified
if deep_get(self.compiler_section, "config", self.compiler):

self.logger.debug(
f"[{self.name}]: Detected compiler: {self.compiler} in 'config' scope overriding default compiler group setting for: {self.compiler_group}"
)
Expand Down
2 changes: 0 additions & 2 deletions buildtest/builders/script.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ def __init__(
numnodes=None,
compiler=None,
):

super().__init__(
name=name,
recipe=recipe,
Expand Down Expand Up @@ -123,7 +122,6 @@ def generate_script(self):
lines += sched_lines

if self.burstbuffer:

burst_buffer_lines = self._get_burst_buffer(self.burstbuffer)
if burst_buffer_lines:
lines += burst_buffer_lines
Expand Down
3 changes: 0 additions & 3 deletions buildtest/builders/spack.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,11 @@ def _spack_environment(self, spack_env):
lines.append(f"spack env rm -y {spack_env['rm']['name']}")

if spack_env.get("create"):

opts = spack_env["create"].get("options") or ""
cmd = ["spack env create", opts]

# create spack environment from name
if spack_env["create"].get("name"):

# if remove_environment is defined we remove the environment before creating it
if spack_env["create"].get("remove_environment"):
lines.append(f"spack env rm -y {spack_env['create']['name']}")
Expand Down Expand Up @@ -246,7 +244,6 @@ def _spack_environment(self, spack_env):

# activate spack environment via directory 'spack env activate -d <dir>'
elif spack_env["activate"].get("dir"):

env_dir = resolve_path(spack_env["activate"]["dir"], exist=False)
if not env_dir:
raise BuildTestError(
Expand Down
5 changes: 0 additions & 5 deletions buildtest/buildsystem/builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ def __init__(
return

if deep_get(self.filters, "maintainers"):

if not self.bp.recipe.get("maintainers"):
console.print(
f"{self.bp.buildspec}: skipping test because [italic]'maintainers'[/italic] field is not specified in buildspec."
Expand Down Expand Up @@ -112,7 +111,6 @@ def __init__(
# Add the builder for the script or spack schema

if recipe["type"] in ["script", "compiler", "spack"]:

builders = self.build(name, recipe)
if builders:
self.builders += builders
Expand Down Expand Up @@ -202,7 +200,6 @@ def create_script_builders(
def create_compiler_builders(
self, name, recipe, executor, nodes=None, procs=None, compiler_name=None
):

"""Create builder objects by calling :class:`buildtest.builders.compiler.CompilerBuilder` class.
args:
Expand Down Expand Up @@ -403,7 +400,6 @@ def build(self, name, recipe):
for compiler_pattern in recipe["compilers"]["name"]:
for bc_name in discovered_compilers:
if re.match(compiler_pattern, bc_name):

builder = self.generate_builders(
name=name, recipe=recipe, compiler_name=bc_name
)
Expand Down Expand Up @@ -463,7 +459,6 @@ def _skip_tests_by_type(self, recipe, name):
"""

if self.filters.get("type"):

found = self.filters["type"] == recipe["type"]

if not found:
Expand Down
1 change: 0 additions & 1 deletion buildtest/buildsystem/checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ def returncode_check(builder):

# if 'returncode' field set for 'status' check the returncode if its not set we return False
if "returncode" in builder.status.keys():

# returncode can be an integer or list of integers
buildspec_returncode = builder.status["returncode"]

Expand Down
1 change: 0 additions & 1 deletion buildtest/buildsystem/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,6 @@ def validate(self):
# with sub schema

for test in self.get_test_names():

self.logger.info(
"Validating test - '%s' in recipe: %s" % (test, self.buildspec)
)
Expand Down
26 changes: 15 additions & 11 deletions buildtest/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,6 @@ def valid_time(value):


def get_parser():

epilog_str = f"""
References
Expand Down Expand Up @@ -530,16 +529,6 @@ def build_menu(subparsers):
type=str,
help="Specify project account used to charge batch jobs (applicable for batch jobs only)",
)
extra_group.add_argument(
"--disable-executor-check",
action="store_false",
help="Disable executor check during configuration check. By default these checks are enforced for Local, Slurm, PBS, LSF, and Cobalt Executor.",
)
extra_group.add_argument(
"--remove-stagedir",
action="store_true",
help="Remove stage directory after job completion.",
)
batch_group.add_argument(
"--maxpendtime",
type=positive_number,
Expand All @@ -562,6 +551,21 @@ def build_menu(subparsers):
nargs="+",
type=positive_number,
)
extra_group.add_argument(
"--disable-executor-check",
action="store_false",
help="Disable executor check during configuration check. By default these checks are enforced for Local, Slurm, PBS, LSF, and Cobalt Executor.",
)
extra_group.add_argument(
"--limit",
type=positive_number,
help="Limit number of tests that can be run.",
)
extra_group.add_argument(
"--remove-stagedir",
action="store_true",
help="Remove stage directory after job completion.",
)
extra_group.add_argument(
"--rebuild",
type=positive_number,
Expand Down
31 changes: 20 additions & 11 deletions buildtest/cli/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@

logger = logging.getLogger(__name__)


# Context manager that copies stdout and any exceptions to a log file
class Tee(object):
def __init__(self, filename):
Expand Down Expand Up @@ -188,7 +189,6 @@ def discover_buildspecs(
# if user pass buildspecs to be excluded (buildtest build -x <buildspec>) then
# discover all excluded buildspecs and remove from discovered list
if exclude_buildspecs:

# discover all excluded buildspecs, if its file add to list,
# if its directory traverse all .yml files
for name in exclude_buildspecs:
Expand Down Expand Up @@ -242,7 +242,6 @@ def print_discovered_buildspecs(buildspec_dict):

# if any buildspecs removed due to -x option we print them to screen
if buildspec_dict["excluded"]:

table = Table(
title="Excluded buildspecs", box=box.DOUBLE_EDGE, header_style="blue"
)
Expand All @@ -254,7 +253,6 @@ def print_discovered_buildspecs(buildspec_dict):

# print breakdown of buildspecs by tags
if buildspec_dict.get("tags"):

for tagname in buildspec_dict["tags"].keys():
table = Table(
title=f"Buildspecs By Tag={tagname}",
Expand All @@ -268,7 +266,6 @@ def print_discovered_buildspecs(buildspec_dict):

# print breakdown of buildspecs by executors
if buildspec_dict.get("executors"):

for executorname in buildspec_dict["executors"].keys():
table = Table(
title=f"Buildspecs by Executor={executorname}",
Expand Down Expand Up @@ -319,7 +316,6 @@ def discover_buildspecs_by_tags(buildspec_cache, tagnames):

for buildspecfile in buildspec_cache["buildspecs"].keys():
for test in buildspec_cache["buildspecs"][buildspecfile].keys():

# if input tag is not of type str we skip the tag name since it is not valid
if not isinstance(name, str):
logger.warning(f"Tag: {name} is not of type 'str'")
Expand Down Expand Up @@ -366,7 +362,6 @@ def discover_buildspecs_by_executor(buildspec_cache, executors):

for buildspecfile in buildspec_cache["buildspecs"].keys():
for test in buildspec_cache["buildspecs"][buildspecfile].keys():

# check if executor in buildspec matches one in argument (buildtest build --executor <EXECUTOR>)
if name == buildspec_cache["buildspecs"][buildspecfile][test].get(
"executor"
Expand Down Expand Up @@ -500,6 +495,7 @@ def __init__(
rerun=None,
executor_type=None,
timeout=None,
limit=None,
):
"""The initializer method is responsible for checking input arguments for type
check, if any argument fails type check we raise an error. If all arguments pass
Expand Down Expand Up @@ -531,6 +527,7 @@ def __init__(
rerun (bool, optional): Rerun last successful **buildtest build** command. This is specified via ``buildtest build --rerun``. All other options will be ignored and buildtest will read buildtest options from file **BUILDTEST_RERUN_FILE**.
executor_type (bool, optional): Filter test by executor type. This option will filter test after discovery by local or batch executors. This can be specified via ``buildtest build --exec-type``
timeout (int, optional): Test timeout in seconds specified by ``buildtest build --timeout``
limit (int, optional): Limit number of tests that can be run. This option is specified by ``buildtest build --limit``
"""

if buildspecs and not isinstance(buildspecs, list):
Expand Down Expand Up @@ -568,6 +565,12 @@ def __init__(
if timeout <= 0:
raise BuildTestError("Timeout must be greater than 0")

if limit is not None:
if not isinstance(limit, int):
raise BuildTestError(f"{timeout} is not of type int")
if limit <= 0:
raise BuildTestError("Limit must be greater than 0")

self.remove_stagedir = remove_stagedir
self.configuration = configuration
self.buildspecs = buildspecs
Expand All @@ -590,6 +593,7 @@ def __init__(
self.numnodes = numnodes
self.executor_type = executor_type
self.timeout = timeout
self.limit = limit

# this variable contains the detected buildspecs that will be processed by buildtest.
self.detected_buildspecs = None
Expand All @@ -614,7 +618,6 @@ def __init__(
)

if self.logdir:

create_dir(self.logdir)
self.logfile.name = os.path.join(
self.logdir, os.path.basename(self.logfile.name)
Expand Down Expand Up @@ -677,7 +680,8 @@ def __init__(
def load_rerun_file(self):
"""This will load content of file BUILDTEST_RERUN_FILE that contains a dictionary of key/value pair
that keeps track of last ``buildtest build`` command. This is used with ``buildtest build --rerun``. Upon loading
file we reinitalize all class variables that store argument for ``buildtest build`` options"""
file we reinitalize all class variables that store argument for ``buildtest build`` options
"""

if not is_file(BUILDTEST_RERUN_FILE):
raise BuildTestError(
Expand Down Expand Up @@ -715,6 +719,7 @@ def load_rerun_file(self):
self.numprocs = content["numprocs"]
self.executor_type = content["executor_type"]
self.timeout = content["timeout"]
self.limit = content["limit"]

def save_rerun_file(self):
"""Record buildtest command options and save them into rerun file which is read when invoking ``buildtest build --rerun``."""
Expand All @@ -741,6 +746,7 @@ def save_rerun_file(self):
"numnodes": self.numnodes,
"executor_type": self.executor_type,
"timeout": self.timeout,
"limit": self.limit,
}

with open(BUILDTEST_RERUN_FILE, "w") as fd:
Expand Down Expand Up @@ -806,6 +812,12 @@ def build(self):
if not self.builders or self.stage == "parse":
return

if self.limit:
self.builders = self.builders[: self.limit]
console.print(
f"[red]Limit number of tests to {self.limit} for Building and Running. "
)

self.build_phase()

# if --stage=build is set we return from method
Expand Down Expand Up @@ -894,14 +906,12 @@ def parse_buildspecs(self):
console.print(f"[red]Invalid Buildspecs: {len(self.invalid_buildspecs)}")

for buildspec in valid_buildspecs:

msg = f"[green]{buildspec}: VALID"
console.print(msg)

# print any skipped buildspecs if they failed to validate during build stage
if self.invalid_buildspecs:
for buildspec in self.invalid_buildspecs:

msg = f"[red]{buildspec}: INVALID"
console.print(msg)

Expand Down Expand Up @@ -1322,7 +1332,6 @@ def print_batch_builders(self, builders):
def print_builders(
self, compiler_builder, spack_builder, script_builder, batch_builder
):

"""Print detected builders during build phase"""

self.print_builders_by_type(script_builder, builder_type="script")
Expand Down
Loading

0 comments on commit 4c4b42b

Please sign in to comment.