Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add support for limiting builds via 'buildtest build --limit' #1381

Merged
merged 7 commits into from
Feb 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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