From 341857fa5ab467b87e00ed0af34c73f1a19587da Mon Sep 17 00:00:00 2001 From: Paul Ferrell <51765748+Paul-Ferrell@users.noreply.github.com> Date: Tue, 5 Sep 2023 08:17:54 -0600 Subject: [PATCH] Fixed permissions issue (#686) * Fixed permissions issue When searching config dirs, not having write directory to a directory causes an 'IsADirectory' error instead of a permissions error. * Several bug fixes, plus a new style test - A few bug fixes fron Nick and Jen - A style test that checks for continued strings with bad spacing. - Fixes for all bad spacing cases found. * Update errors.py --- lib/pavilion/arguments.py | 3 +- lib/pavilion/builder.py | 2 +- lib/pavilion/commands/_run.py | 4 +- lib/pavilion/commands/build.py | 2 +- lib/pavilion/commands/config.py | 6 +- lib/pavilion/commands/graph.py | 4 +- lib/pavilion/commands/list_cmd.py | 4 +- lib/pavilion/commands/ls.py | 2 +- lib/pavilion/commands/result.py | 6 +- lib/pavilion/commands/run.py | 4 +- lib/pavilion/commands/show.py | 6 +- lib/pavilion/commands/view.py | 2 +- lib/pavilion/config.py | 13 ++-- lib/pavilion/errors.py | 2 +- lib/pavilion/expression_functions/base.py | 4 +- lib/pavilion/filters.py | 4 +- lib/pavilion/parsers/expressions.py | 26 +++---- lib/pavilion/resolver/proto_test.py | 2 +- lib/pavilion/resolver/request.py | 8 +-- lib/pavilion/result_parsers/base_classes.py | 10 +-- lib/pavilion/result_parsers/filecheck.py | 2 +- lib/pavilion/result_parsers/json.py | 14 ++-- lib/pavilion/schedulers/advanced.py | 4 +- lib/pavilion/schedulers/config.py | 10 +-- lib/pavilion/schedulers/plugins/flux.py | 6 +- lib/pavilion/schedulers/plugins/slurm.py | 8 +-- lib/pavilion/schedulers/scheduler.py | 8 +-- lib/pavilion/series/series.py | 2 +- lib/pavilion/series/test_set.py | 2 +- lib/pavilion/test_config/file_format.py | 20 +++--- lib/pavilion/test_run/test_run.py | 2 +- lib/pavilion/variables.py | 2 +- lib/yaml_config/structures.py | 6 +- test/tests/style_tests.py | 76 +++++++++++++++++++-- 34 files changed, 172 insertions(+), 104 deletions(-) diff --git a/lib/pavilion/arguments.py b/lib/pavilion/arguments.py index 7d5a14420..2190ffdf4 100644 --- a/lib/pavilion/arguments.py +++ b/lib/pavilion/arguments.py @@ -77,8 +77,7 @@ def get_parser(): '--profile-sort', default=PROFILE_SORT_DEFAULT, choices=['cumtime', 'calls', 'file', 'line', 'name', 'nfl', 'time'], help="The sort method for the profile table. See:\n" - "https://docs.python.org/3.5/library/profile.html" - "#pstats.Stats.sort_stats") + "https://docs.python.org/3.5/library/profile.html#pstats.Stats.sort_stats") parser.add_argument( '--profile-count', default=PROFILE_COUNT_DEFAULT, action='store', type=int, diff --git a/lib/pavilion/builder.py b/lib/pavilion/builder.py index 42672e9a0..dd701f196 100644 --- a/lib/pavilion/builder.py +++ b/lib/pavilion/builder.py @@ -975,7 +975,7 @@ def __hash__(self): def __eq__(self, other): if not isinstance(other, TestBuilder): - raise ValueError("Can only compare a builder instance with another" + raise ValueError("Can only compare a builder instance with another " "builder instance.") compare_keys = [ diff --git a/lib/pavilion/commands/_run.py b/lib/pavilion/commands/_run.py index 0d561429f..c2fe44824 100644 --- a/lib/pavilion/commands/_run.py +++ b/lib/pavilion/commands/_run.py @@ -22,7 +22,7 @@ def __init__(self): super().__init__( '_run', - 'Setup and run a single test, under the assumption we\'re already' + 'Setup and run a single test, under the assumption we\'re already ' 'in the expected, scheduled environment.') def _setup_arguments(self, parser): @@ -171,7 +171,7 @@ def _run(self, test: TestRun): fprint(sys.stdout, "Unexpected error gathering results.", err) test.status.set(STATES.RESULTS_ERROR, "Unexpected error parsing results: '{}'... (This is a " - "bug, you should report it.)" + "bug, you should report it.) " "See 'pav log kickoff {}' for the full error." .format(str(err)[:100], test.id)) raise diff --git a/lib/pavilion/commands/build.py b/lib/pavilion/commands/build.py index 94d1dea1a..e091201a5 100644 --- a/lib/pavilion/commands/build.py +++ b/lib/pavilion/commands/build.py @@ -18,7 +18,7 @@ def __init__(self): self, name="build", description="Perform just the build step on the given tests, " - "or perform rebuilds. May still use the scheduler" + "or perform rebuilds. May still use the scheduler " "to build tests that specify they must be built on " "nodes.", short_help="Build and re-build.", diff --git a/lib/pavilion/commands/config.py b/lib/pavilion/commands/config.py index 9ab38cd63..f3a5ceae9 100644 --- a/lib/pavilion/commands/config.py +++ b/lib/pavilion/commands/config.py @@ -51,7 +51,7 @@ def _setup_arguments(self, parser): create_p.add_argument( '--group', - help="Set the group of the created directory to this group name, and set the" + help="Set the group of the created directory to this group name, and set the " "group sticky bit to the config dir and it's working dir (if given).") create_p.add_argument( @@ -71,7 +71,7 @@ def _setup_arguments(self, parser): ) setup_p.add_argument( '--group', - help="Set the group of the created directory to this group name, and set the" + help="Set the group of the created directory to this group name, and set the " "group sticky bit to the config dir and it's working directory.") setup_p.add_argument( 'path', type=Path, @@ -90,7 +90,7 @@ def _setup_arguments(self, parser): remove_p = subparsers.add_parser( 'remove', help="Remove the given config dir path from the root pavilion.yaml", - description="Remove the config config dir path from the root pavilion.yaml. The" + description="Remove the config config dir path from the root pavilion.yaml. The " "directory itself is not removed.") remove_p.add_argument("config", help="Path or config label to remove.") diff --git a/lib/pavilion/commands/graph.py b/lib/pavilion/commands/graph.py index 434fac2d6..5ceece069 100644 --- a/lib/pavilion/commands/graph.py +++ b/lib/pavilion/commands/graph.py @@ -40,12 +40,12 @@ def __init__(self): super().__init__( 'graph', description=( - "Produce a graph from a set of test results. Each x value for" + "Produce a graph from a set of test results. Each x value for " "each test run matched will be plotted. You can graph multiple " "result values , but it's up to you to make sure units and the " "plot as a whole makes sense. Wildcards in the x and y " "specifiers can be used to plot more complicated graphs." - ".\n\n" + "\n\n" "Graph Command Instructions:\n" " 1. Determine which test run results you want to\n" " graph. Provide these test runs specifically\n" diff --git a/lib/pavilion/commands/list_cmd.py b/lib/pavilion/commands/list_cmd.py index 1a5440b2d..6091c6c73 100644 --- a/lib/pavilion/commands/list_cmd.py +++ b/lib/pavilion/commands/list_cmd.py @@ -73,7 +73,7 @@ def _setup_arguments(self, parser): parser.add_argument( '--out-fields', '-O', - help="A comma separated list of fields to put in the output. More" + help="A comma separated list of fields to put in the output. More " "than one field requires '--long' or '--csv' mode. " "See --show-fields for available fields." ) @@ -108,7 +108,7 @@ def _setup_arguments(self, parser): runs_p.add_argument( 'tests', nargs="*", default=['all'], - help="Specific tests (or series) to filter from. Defaults to" + help="Specific tests (or series) to filter from. Defaults to " "'all'." ) diff --git a/lib/pavilion/commands/ls.py b/lib/pavilion/commands/ls.py index 385644d5d..9bcdb3ce1 100644 --- a/lib/pavilion/commands/ls.py +++ b/lib/pavilion/commands/ls.py @@ -64,7 +64,7 @@ def _setup_arguments(self, parser): parser.add_argument( '--tree', action='store_true', - help="List file tree for the given test run (may be huge). Most output args don't" + help="List file tree for the given test run (may be huge). Most output args don't " "apply.", ) parser.add_argument( diff --git a/lib/pavilion/commands/result.py b/lib/pavilion/commands/result.py index 6b8b2d697..2bc70f390 100644 --- a/lib/pavilion/commands/result.py +++ b/lib/pavilion/commands/result.py @@ -45,7 +45,7 @@ def _setup_arguments(self, parser): group = parser.add_mutually_exclusive_group() group.add_argument( "-k", "--key", type=str, default='', - help="Comma separated list of additional result keys to display." + help="Comma separated list of additional result keys to display. " "Use ~ (tilda) in front of default key to remove from default list." ) group.add_argument( @@ -74,7 +74,7 @@ def _setup_arguments(self, parser): ) parser.add_argument( '-L', '--show-log', action='store_true', default=False, - help="Also show the result processing log. This is particularly" + help="Also show the result processing log. This is particularly " "useful when re-parsing results, as the log is not saved." ) @@ -175,7 +175,7 @@ def run(self, pav_cfg, args): else: if len(results) > 1: output.fprint(self.errfile, - "Please give a single test id when requesting the full" + "Please give a single test id when requesting the full " "result log.", color=output.YELLOW) return 1 diff --git a/lib/pavilion/commands/run.py b/lib/pavilion/commands/run.py index 9739c63b2..10afa8714 100644 --- a/lib/pavilion/commands/run.py +++ b/lib/pavilion/commands/run.py @@ -64,13 +64,13 @@ def _generic_arguments(parser): parser.add_argument( '-o', '--os', action='store', help='The operating system to configure this test for. If not ' - 'specified, the current operating system as denoted by the sys' + 'specified, the current operating system as denoted by the sys ' 'plugin \'sys_os\' is used.') parser.add_argument( '-H', '--host', action='store', help='The host to configure this test for. If not specified, the ' 'current host as denoted by the sys plugin \'sys_host\' is ' - 'used. Host configurations are overlayed on operating system' + 'used. Host configurations are overlayed on operating system ' 'configurations.') parser.add_argument( '-n', '--name', action='store', default='' diff --git a/lib/pavilion/commands/show.py b/lib/pavilion/commands/show.py index 7354e8411..ec9526bc3 100644 --- a/lib/pavilion/commands/show.py +++ b/lib/pavilion/commands/show.py @@ -90,7 +90,7 @@ def _setup_arguments(self, parser): 'functions', aliases=['func', 'function'], help="Show available expression functions plugins.", - description="Expression function plugins allow you to dynamically" + description="Expression function plugins allow you to dynamically " "add complex (or simple) new functionality to " "Pavilion value expressions.") func_group.add_argument( @@ -345,9 +345,9 @@ def _setup_arguments(self, parser): help="Show the available system variables.", description="System variables are available for use in test " "configurations. Simply put the name in curly " - "brackets in any config value. '{sys_name}' or '{" + "brackets in any config value. '{sys_name}' or '{ " "sys.sys_name}'. You can add your own " - "system_variables via plugins. They may be deferred" + "system_variables via plugins. They may be deferred " "(resolved on nodes, at test run time), otherwise " "they are resolved at test kickoff time on the " "kickoff host." diff --git a/lib/pavilion/commands/view.py b/lib/pavilion/commands/view.py index 3e6e495bf..2215ac391 100644 --- a/lib/pavilion/commands/view.py +++ b/lib/pavilion/commands/view.py @@ -55,7 +55,7 @@ def _setup_arguments(self, parser): ) parser.add_argument( 'tests', action='store', nargs='*', - help='The name of the test to view. Should be in the format' + help='The name of the test to view. Should be in the format ' '..') SLEEP_INTERVAL = 1 diff --git a/lib/pavilion/config.py b/lib/pavilion/config.py index 6fcd6974f..4c3314840 100644 --- a/lib/pavilion/config.py +++ b/lib/pavilion/config.py @@ -347,7 +347,7 @@ class PavilionConfigLoader(yc.YamlConfigLoader): ExPathElem( "working_dir", default=DEFAULT_WORKING_DIR, - help_text="Set the default working directory. Each config dir can set its" + help_text="Set the default working directory. Each config dir can set its " "own working_dir."), ExPathElem( 'spack_path', default=None, required=False, @@ -381,7 +381,7 @@ class PavilionConfigLoader(yc.YamlConfigLoader): ), yc.IntRangeElem( "max_cpu", default=NCPU, vmin=1, - help_text="Maximum number of cpus to use when spawning multiple processes." + help_text="Maximum number of cpus to use when spawning multiple processes. " "The number used may be less depending on the task." ), yc.StrElem( @@ -389,8 +389,7 @@ class PavilionConfigLoader(yc.YamlConfigLoader): default=LOG_FORMAT, help_text="The log format to use for the pavilion logger. " "Uses the modern '{' format style. See: " - "https://docs.python.org/3/library/logging.html#" - "logrecord-attributes"), + "https://docs.python.org/3/library/logging.html#logrecord-attributes"), yc.StrElem( "log_level", default="info", choices=['debug', 'info', 'warning', 'error', 'critical'], @@ -542,7 +541,11 @@ def add_config_dirs(pav_cfg, setup_working_dirs: bool) -> OrderedDict: continue # Attempt to force a permissions error if we can't read this directory. - config_dir.touch() + try: + config_dir.touch() + except IsADirectoryError: + raise PermissionError("Couldn't read directory '{}'".format(config_dir)) + list(config_dir.iterdir()) except PermissionError: diff --git a/lib/pavilion/errors.py b/lib/pavilion/errors.py index 10581ced8..e8cdc7b13 100644 --- a/lib/pavilion/errors.py +++ b/lib/pavilion/errors.py @@ -60,7 +60,7 @@ def pformat(self) -> str: if self.data: data = pprint.pformat(self.data, width=width - tab_level*2) for line in data.split('\n'): - lines.extend(tab_level*self.TAB_LEVEL + line) + lines.append(tab_level*self.TAB_LEVEL + line) while next_exc: tab_level += 1 diff --git a/lib/pavilion/expression_functions/base.py b/lib/pavilion/expression_functions/base.py index 92a2496b0..35d111c22 100644 --- a/lib/pavilion/expression_functions/base.py +++ b/lib/pavilion/expression_functions/base.py @@ -112,7 +112,7 @@ class docstring is used by default. else: if len(sig.parameters) != len(arg_specs): raise FunctionPluginError( - "Invalid arg specs. The function takes {} arguments, but" + "Invalid arg specs. The function takes {} arguments, but " "an arg_spec of length {} was provided." .format(len(sig.parameters), len(arg_specs))) @@ -333,7 +333,7 @@ def activate(self): self.name, self.path, other.path) else: raise RuntimeError( - "Function plugin conflict. Parser '{}' at '{}'" + "Function plugin conflict. Parser '{}' at '{}' " "has the same priority as plugin at '{}'" .format(self.name, self.path, other.path)) diff --git a/lib/pavilion/filters.py b/lib/pavilion/filters.py index 115b18f4b..f78aedc50 100644 --- a/lib/pavilion/filters.py +++ b/lib/pavilion/filters.py @@ -81,7 +81,7 @@ def add_common_filter_args(target: str, if 'has-state' not in disable_opts: arg_parser.add_argument( '--has-state', type=str, default=defaults['state'], - help="Include only {} who have had the given state at some point." + help="Include only {} who have had the given state at some point. " "Default: {}".format(target, defaults['has_state']) ) arg_parser.add_argument( @@ -115,7 +115,7 @@ def add_common_filter_args(target: str, arg_parser.add_argument( '--newer-than', type=utils.hr_cutoff_to_ts, default=defaults['newer_than'], - help='As per older-than, but include only {} newer than the given' + help='As per older-than, but include only {} newer than the given ' 'time. Default: {}'.format(target, defaults['newer_than']) ) if 'state' not in disable_opts: diff --git a/lib/pavilion/parsers/expressions.py b/lib/pavilion/parsers/expressions.py index 95fd1deed..2ace3d34e 100644 --- a/lib/pavilion/parsers/expressions.py +++ b/lib/pavilion/parsers/expressions.py @@ -19,31 +19,31 @@ // All expressions will resolve to the start expression. start: expr _WS? - | // An empty string is valid - + | // An empty string is valid + // Trailing whitespace is ignored. Whitespace between tokens is // ignored below. _WS: /\s+/ expr: or_expr -// These set order of operations. +// These set order of operations. // See https://en.wikipedia.org/wiki/Operator-precedence_parser -or_expr: and_expr ( OR and_expr )* +or_expr: and_expr ( OR and_expr )* and_expr: not_expr ( AND not_expr )* -not_expr: NOT? compare_expr +not_expr: NOT? compare_expr compare_expr: add_expr ((EQ | NOT_EQ | LT | GT | LT_EQ | GT_EQ ) add_expr)* add_expr: mult_expr ((PLUS | MINUS) mult_expr)* mult_expr: pow_expr ((TIMES | DIVIDE | INT_DIV | MODULUS) pow_expr)* pow_expr: primary ("^" primary)? -primary: literal - | var_ref +primary: literal + | var_ref | negative | "(" expr ")" | function_call | list_ -// A function call can contain zero or more arguments. +// A function call can contain zero or more arguments. function_call: NAME "(" (expr ("," expr)*)? ")" negative: (MINUS|PLUS) primary @@ -53,7 +53,7 @@ | FLOAT | BOOL | ESCAPED_STRING - + // Allows for trailing commas list_: L_BRACKET (expr ("," expr)* ","?)? R_BRACKET @@ -92,11 +92,11 @@ // This will be prioritized over 'NAME' matches BOOL.2: "True" | "False" -// Names can be lower-case or capitalized, but must start with a letter or +// Names can be lower-case or capitalized, but must start with a letter or // underscore NAME.1: /[a-zA-Z_][a-zA-Z0-9_]*/ -// Ignore all whitespace between tokens. +// Ignore all whitespace between tokens. %ignore / +(?=[^.])/ ''' @@ -614,7 +614,7 @@ def _resolve_ref(self, base, key_parts: list, seen_parts: tuple = tuple(), if idx >= len(base): raise ValueError( - "Key component '{}' is out of range for the list" + "Key component '{}' is out of range for the list " "at '{}'.".format(idx, '.'.join(seen_parts)) ) @@ -631,7 +631,7 @@ def _resolve_ref(self, base, key_parts: list, seen_parts: tuple = tuple(), return self._resolve_ref(base[key_part], key_parts, seen_parts, allow_listing) - raise ValueError("Key component '{}' given, but value '{}' at '{}'" + raise ValueError("Key component '{}' given, but value '{}' at '{}' " "is a '{}' not a dict or list." .format(key_part, base, '.'.join(seen_parts), type(base))) diff --git a/lib/pavilion/resolver/proto_test.py b/lib/pavilion/resolver/proto_test.py index 84b2ea7ce..65681c586 100644 --- a/lib/pavilion/resolver/proto_test.py +++ b/lib/pavilion/resolver/proto_test.py @@ -52,7 +52,7 @@ def resolve(self) -> Dict: name = self.config['name'] permute_on = self.config.get('permute_on') if permute_on: - permute_values = {key: var_man.get(key) for key in permute_on} + permute_values = {key: self.var_man.get(key) for key in permute_on} raise TestConfigError( "Error resolving test {} with permute values:" diff --git a/lib/pavilion/resolver/request.py b/lib/pavilion/resolver/request.py index 19be68051..86decd251 100644 --- a/lib/pavilion/resolver/request.py +++ b/lib/pavilion/resolver/request.py @@ -31,10 +31,10 @@ def __init__(self, request_str): match = self.REQUEST_RE.match(request_str) if not match: raise TestConfigError( - "Test requests must be in the form 'suite_name', 'suite_name.test_name', or\n" - "'suite_name.test_name.permutation_name. They may be preceeded by a repeat\n" - "multiplier (e.g. '5*'). test_name and permutation_name can also use globbing\n" - "syntax (*, ?, []). For example, 'suite.test*.perm-[abc]." + "Test requests must be in the form 'suite_name', 'suite_name.test_name', or " + "'suite_name.test_name.permutation_name. They may be preceeded by a repeat " + "multiplier (e.g. '5*'). test_name and permutation_name can also use globbing " + "syntax (*, ?, []). For example, 'suite.test*.perm-[abc].\n" "Got: {}".format(request_str)) pre_count, self.suite, self.test, self.permutation, post_count = match.groups() diff --git a/lib/pavilion/result_parsers/base_classes.py b/lib/pavilion/result_parsers/base_classes.py index c125fa6b6..2dd042b0c 100644 --- a/lib/pavilion/result_parsers/base_classes.py +++ b/lib/pavilion/result_parsers/base_classes.py @@ -157,7 +157,7 @@ def __init__(self, name, description, defaults=None, (isinstance(elem, yc.ListElem) and isinstance(elem._sub_elem, yc.StrElem))): raise RuntimeError( - "Config elements for result parsers must be strings" + "Config elements for result parsers must be strings " "or lists of strings. Got elem {} in parser at {}" .format(elem, self.path) ) @@ -165,7 +165,7 @@ def __init__(self, name, description, defaults=None, if elem.default not in [None, []] or elem._choices is not None: raise RuntimeError( "Config elements for result parsers shouldn't set " - "a default or choices (use the defaults or validators" + "a default or choices (use the defaults or validators " "argument for the result_parser init). Problem found " "in {} in result parser at {}" .format(elem, self.path) @@ -311,7 +311,7 @@ def check_args(self, **kwargs) -> dict: "files", sub_elem=yc.StrElem(), help_text="Path to the file/s that this result parser " - "will examine. Each may be a file glob," + "will examine. Each may be a file glob, " "such as '*.log'"), yc.StrElem( "per_file", @@ -354,7 +354,7 @@ def check_args(self, **kwargs) -> dict: "preceded_by", sub_elem=yc.StrElem(), help_text=( "A list of regular expressions that must match lines that " - "precede the lines to be parsed. Empty items" + "precede the lines to be parsed. Empty items " "in the sequence will match anything. The result parser " "will see the file starting from start of the line after " "these match.")), @@ -569,7 +569,7 @@ def activate(self): "Result parser '%s' at %s is ignored in lieu of %s.", self.name, self.path, other.path) else: - raise RuntimeError("Result parser conflict. Parser '{}' at {}" + raise RuntimeError("Result parser conflict. Parser '{}' at {} " "has the same priority as {}" .format(self.name, other.path, self.path)) else: diff --git a/lib/pavilion/result_parsers/filecheck.py b/lib/pavilion/result_parsers/filecheck.py index 7974d0295..33c0029bd 100644 --- a/lib/pavilion/result_parsers/filecheck.py +++ b/lib/pavilion/result_parsers/filecheck.py @@ -11,7 +11,7 @@ class Filecheck(base_classes.ResultParser): def __init__(self): super().__init__( name='filecheck', - description="Checks working directory for a given file. Globs are" + description="Checks working directory for a given file. Globs are " "accepted.", config_elems=[], ) diff --git a/lib/pavilion/result_parsers/json.py b/lib/pavilion/result_parsers/json.py index a362924c6..d39cf399f 100644 --- a/lib/pavilion/result_parsers/json.py +++ b/lib/pavilion/result_parsers/json.py @@ -14,27 +14,27 @@ class Json(base_classes.ResultParser): def __init__(self): super().__init__( name='json', - description="Return a JSON dict parsed from the given file according to" + description="Return a JSON dict parsed from the given file according to " "the given keys. Includes all keys by default.", config_elems=[ yc.ListElem( 'include_only', sub_elem = yc.StrElem(), - help_text="Include this key and exclude all others." + help_text="Include this key and exclude all others. " "Example: '[key1, key2.subkey]'" ), yc.ListElem( 'exclude', sub_elem = yc.StrElem(), - help_text="Exclude this key." + help_text="Exclude this key. " "Example: '[key1, key2.subkey]'" ), yc.StrElem( 'stop_at', - help_text="The line after the final line of JSON to be parsed." - "If the results contain more than just pure JSON, use the preceded_by" - "option to mark where the JSON begins and this option to mark where" - "the JSON ends." + help_text="The line after the final line of JSON to be parsed. " + "If the results contain more than just pure JSON, use the preceded_by " + "option to mark where the JSON begins and this option to mark where " + "the JSON ends. " "Example: \"a string of text\"" ) ] diff --git a/lib/pavilion/schedulers/advanced.py b/lib/pavilion/schedulers/advanced.py index 8483f644a..5a32c5282 100644 --- a/lib/pavilion/schedulers/advanced.py +++ b/lib/pavilion/schedulers/advanced.py @@ -194,8 +194,8 @@ def _get_system_inventory(self, sched_config: dict) -> Union[Nodes, None]: node_info = self._transform_raw_node_data(sched_config, raw_node, extra) if 'name' not in node_info: - raise RuntimeError("Advanced schedulers must always return a node" - "'name' key when transforming raw node data." + raise RuntimeError("Advanced schedulers must always return a node " + "'name' key when transforming raw node data. " "Got: {}".format(node_info)) nodes[node_info['name']] = node_info diff --git a/lib/pavilion/schedulers/config.py b/lib/pavilion/schedulers/config.py index c3227ea2a..28b13bfd8 100644 --- a/lib/pavilion/schedulers/config.py +++ b/lib/pavilion/schedulers/config.py @@ -45,7 +45,7 @@ class ScheduleConfig(yc.KeyedElem): help_text="If true, share the allocation with other tests in the same " "chunk. The allocation will have a number of nodes equal to " "the test that needs the most. Tests started with " - "{{sched.run_cmd}} will start with the right number of nodes." + "{{sched.run_cmd}} will start with the right number of nodes. " "Tests are run one at a time within the allocation. This is " "great for large tests over a many/all nodes, especially on " "large systems where node setup/cleanup takes a while."), @@ -56,15 +56,15 @@ class ScheduleConfig(yc.KeyedElem): "slurm, tasks_per_node becomes the maximum tasks per node."), yc.StrElem( 'tasks_per_node', - help_text="The number of tasks to start per node. This can be" - "an integer, the keyword 'all', or a percentage." + help_text="The number of tasks to start per node. This can be " + "an integer, the keyword 'all', or a percentage. " "'all' will create a number of tasks per node equal " "to the CPUs for the node with the least CPUs in the " "selected nodes. A percentage will create a task " "for that percentage of the 'all', rounded down (min 1)."), yc.StrElem( 'min_tasks_per_node', - help_text="A minimum number of tasks per node. Must be an integer (or undefined)," + help_text="A minimum number of tasks per node. Must be an integer (or undefined), " "This will take precedence over 'tasks_per_node' if it is larger than that " "value after it is calculated."), yc.StrElem( @@ -120,7 +120,7 @@ class ScheduleConfig(yc.KeyedElem): elements=[ yc.StrElem( 'size', - help_text="Divide the allocation into chunks of this many nodes." + help_text="Divide the allocation into chunks of this many nodes. " "A variable 'sched.chunks' will hold a list of " "these chunks that can be permuted over in the test " "config. No values or 0 sets the chunk size to include " diff --git a/lib/pavilion/schedulers/plugins/flux.py b/lib/pavilion/schedulers/plugins/flux.py index 79d28ed2b..44c3031ad 100644 --- a/lib/pavilion/schedulers/plugins/flux.py +++ b/lib/pavilion/schedulers/plugins/flux.py @@ -159,7 +159,7 @@ def _kickoff_lines(self) -> List[str]: lines.append('#flux: --requires={}'.format(constraint)) # Should take times like 30s, 5m, 2h, or 8d - time_limit = '{}'.format(60*self._config['time_limit']) + time_limit = '{}'.format(self._config['time_limit']) lines.append('#flux: -t {}'.format(time_limit)) # Specify the number of nodes @@ -247,8 +247,8 @@ def _get_alloc_nodes(self, job) -> NodeList: # Ensure that this is a child instance depth = child_handle.attr_get("instance-level") if depth == 0: - raise RuntimeError("This function should only be called from" - "inside of an allocation, but it appears" + raise RuntimeError("This function should only be called from " + "inside of an allocation, but it appears " "to have been called from outside of one.") # Get the Job ID of this child instance diff --git a/lib/pavilion/schedulers/plugins/slurm.py b/lib/pavilion/schedulers/plugins/slurm.py index f97d86abb..73299ea90 100644 --- a/lib/pavilion/schedulers/plugins/slurm.py +++ b/lib/pavilion/schedulers/plugins/slurm.py @@ -280,10 +280,10 @@ def _get_config_elems(self): "'sched.test_cmd' variable."), yc.ListElem(name='sbatch_extra', sub_elem=yc.StrElem(), - help_text="Extra arguments to add as sbatch header lines." + help_text="Extra arguments to add as sbatch header lines. " "Example: ['--deadline now+20hours']"), yc.StrElem(name='mpi_cmd', - help_text="What command to use to start mpi jobs. Options" + help_text="What command to use to start mpi jobs. Options " "are {}.".format(self.MPI_CMD_OPTIONS)), ] @@ -345,7 +345,7 @@ def parse_node_list(cls, node_list) -> NodeList: # If all the parts matched, then it's an overall format issue. raise ValueError("Invalid Node List: '{}' " - "Good Example: foo003,foo0[10-20]," + "Good Example: foo003,foo0[10-20], " "foo[103-104], foo[10,12-14]") nodes = [] @@ -749,7 +749,7 @@ def _job_status(self, pav_cfg, job_info: JobInfo) -> TestStatusInfo: # it might be! Who knows. return TestStatusInfo( state=STATES.SCHEDULED, - note="Job '{}' has unknown/unhandled job state '{}'. We have no" + note="Job '{}' has unknown/unhandled job state '{}'. We have no " "idea what is going on.".format(job_info['id'], job_state), when=time.time() ) diff --git a/lib/pavilion/schedulers/scheduler.py b/lib/pavilion/schedulers/scheduler.py index 094641dac..76dc74cee 100644 --- a/lib/pavilion/schedulers/scheduler.py +++ b/lib/pavilion/schedulers/scheduler.py @@ -122,7 +122,7 @@ def __init__(self, name, description, priority=PRIO_CORE): self._job_statuses = JobStatusDict({}) # type: JobStatusDict if self.VAR_CLASS is None: - raise SchedulerPluginError("You must set the Var class for" + raise SchedulerPluginError("You must set the Var class for " "each plugin type.") # These need to be overridden by plugin classes. See the Advanced/Basic classes @@ -132,7 +132,7 @@ def _available(self) -> bool: """Return true if this scheduler is available on this system, false otherwise.""" - raise NotImplementedError("You must add a method to determine if the" + raise NotImplementedError("You must add a method to determine if the " "scheduler is available on this system.") def _job_status(self, pav_cfg, job_info: JobInfo) -> Union[TestStatusInfo, None]: @@ -361,7 +361,7 @@ def get_conf(self) -> Tuple[Union[yc.KeyedElem, None], dict, dict]: for elem in config_elements: if elem.name not in validators: raise SchedulerPluginError( - "Scheduler plugin gave config element '{}', but did not provide" + "Scheduler plugin gave config element '{}', but did not provide " "a validator for that key.".format(elem.name) ) @@ -425,7 +425,7 @@ def _create_kickoff_script_stub(self, pav_cfg, job_name: str, log_path: Path, """ if nodes and node_range: - raise RuntimeError("Only one of the 'nodes' and 'node_range' arguments may be" + raise RuntimeError("Only one of the 'nodes' and 'node_range' arguments may be " "provided.") elif not (nodes or node_range): raise RuntimeError("One of 'nodes' and 'node_range' must be provided.") diff --git a/lib/pavilion/series/series.py b/lib/pavilion/series/series.py index 04ec18dfb..ab43c5e32 100644 --- a/lib/pavilion/series/series.py +++ b/lib/pavilion/series/series.py @@ -254,7 +254,7 @@ def _create_test_sets(self, iteration=0): separately for unit testing.""" if self.test_sets: - raise RuntimeError("_create_test_sets should only run when there are" + raise RuntimeError("_create_test_sets should only run when there are " "no test sets for a series, but this series has: {}" .format(self.test_sets)) diff --git a/lib/pavilion/series/test_set.py b/lib/pavilion/series/test_set.py index 3ab286177..b9267b471 100644 --- a/lib/pavilion/series/test_set.py +++ b/lib/pavilion/series/test_set.py @@ -141,7 +141,7 @@ def __ordered_split(self) -> List['TestSet']: the last and the tests run in the order given.""" if self.tests: - raise RuntimeError("You can't split a TestSet once it's tests have been" + raise RuntimeError("You can't split a TestSet once it's tests have been " "created.") # For empty or single test_name test sets, we don't need to do anything. diff --git a/lib/pavilion/test_config/file_format.py b/lib/pavilion/test_config/file_format.py index de13ff9d5..9b95bbe98 100644 --- a/lib/pavilion/test_config/file_format.py +++ b/lib/pavilion/test_config/file_format.py @@ -501,7 +501,7 @@ class TestConfigLoader(yc.YamlConfigLoader): 'chunk', default='any', help_text="The scheduler chunk to run on. Will run on 'any' chunk " "by default, but the chunk may be specified by number. The " - "available chunk ids are in the sched.chunks variable, and" + "available chunk ids are in the sched.chunks variable, and " "can be permuted on." ), CondCategoryElem( @@ -511,10 +511,10 @@ class TestConfigLoader(yc.YamlConfigLoader): "section evaluate to true. Each clause consists of " "a mapping key (that can contain Pavilion variable " "references, like '{{pav.user}}' or '{{sys.sys_arch}}'" - ") and one or more regex values" + ") and one or more regex values " "(that much match the whole key). A clause is true " - "if the value of the Pavilion variable matches one or" - " more of the values. " + "if the value of the Pavilion variable matches one or " + "more of the values. " ), CondCategoryElem( 'not_if', sub_elem=yc.ListElem(sub_elem=yc.StrElem()), @@ -524,7 +524,7 @@ class TestConfigLoader(yc.YamlConfigLoader): "a mapping key (that can contain Pavilion variable " "references, like '{{pav.user}}' or " "'{{sys.sys_arch}}') and one or more " - "regex values (that much match the whole key)." + "regex values (that much match the whole key). " "A clause is true if the value of " "the Pavilion variable matches one or more of the " " values." @@ -587,14 +587,14 @@ class TestConfigLoader(yc.YamlConfigLoader): yc.ListElem( 'copy_files', sub_elem=yc.StrElem(), help_text="When attaching the build to a test run, copy " - "these files instead of creating a symlink." + "these files instead of creating a symlink. " "They may include path glob wildcards, " "including the recursive '**'."), PathCategoryElem( 'create_files', key_case=PathCategoryElem.KC_MIXED, sub_elem=yc.ListElem(sub_elem=yc.StrElem()), - help_text="File(s) to create at path relative to the test's" + help_text="File(s) to create at path relative to the test's " "build/run directory. The key is the filename, " "while the contents is a list of lines to include in the " "file. Pavilion test variables will be resolved in this lines " @@ -728,7 +728,7 @@ class TestConfigLoader(yc.YamlConfigLoader): 'create_files', key_case=PathCategoryElem.KC_MIXED, sub_elem=yc.ListElem(sub_elem=yc.StrElem()), - help_text="File(s) to create at path relative to the test's" + help_text="File(s) to create at path relative to the test's " "build/run directory. The key is the filename, " "while the contents is a list of lines to include in the " "file. Pavilion test variables will be resolved in this lines " @@ -814,7 +814,7 @@ class TestConfigLoader(yc.YamlConfigLoader): elements=[ yc.ListElem( 'modules', sub_elem=yc.StrElem(), - help_text="Modules to load/remove/swap when the given module" + help_text="Modules to load/remove/swap when the given module " "is specified. Loads and swaps into the wrapped module " "will automatically be at the requested version if none " "is given. (IE - If the user asks for gcc/5.2, a " @@ -902,7 +902,7 @@ def add_result_parser_config(cls, name, config_items): list_elem = ResultParserCatElem(name, sub_elem=config) if name in [e.name for e in cls._RESULT_PARSERS.config_elems.values()]: - raise ValueError("Tried to add result parser with name '{}'" + raise ValueError("Tried to add result parser with name '{}' " "to the config, but one already exists." .format(name)) diff --git a/lib/pavilion/test_run/test_run.py b/lib/pavilion/test_run/test_run.py index 3231da6a5..22ad5a1bc 100644 --- a/lib/pavilion/test_run/test_run.py +++ b/lib/pavilion/test_run/test_run.py @@ -838,7 +838,7 @@ def gather_results(self, run_result: int, regather: bool = False, """ if self.finished is None: raise RuntimeError( - "test.gather_results can't be run unless the test was run" + "test.gather_results can't be run unless the test was run " "(or an attempt was made to run it. " "This occurred for test {s.name}, #{s.id}" .format(s=self)) diff --git a/lib/pavilion/variables.py b/lib/pavilion/variables.py index 806a007f0..680f14c9a 100644 --- a/lib/pavilion/variables.py +++ b/lib/pavilion/variables.py @@ -340,7 +340,7 @@ def resolve_references(self, partial=False, skip_deps: List = None) \ try: r_var_set, r_var, r_index, r_subvar = ref_key = self.resolve_key(var_str) except KeyError as err: - raise VariableError("Key '{}'referenced by user variable '{}' could " + raise VariableError("Key '{}' referenced by user variable '{}' could " "not be parsed.".format(var_str, uvar), prior_error=err) diff --git a/lib/yaml_config/structures.py b/lib/yaml_config/structures.py index 9a73399b3..732128896 100644 --- a/lib/yaml_config/structures.py +++ b/lib/yaml_config/structures.py @@ -91,7 +91,7 @@ def normalize(self, value, root_name=None): if root_name is not None: name = root_name - else: + else: name = self.name if value is None or value == [None]: @@ -816,7 +816,7 @@ def find(self, dotted_key): next_key = parts[1] if len(parts) == 2 else '' if key != '*': raise KeyError( - "Invalid dotted key for {} called '{}'. CategoryElem" + "Invalid dotted key for {} called '{}'. CategoryElem " "must have their element name given as a *, since it's " "sub-element isn't named. Got '{}' from '{}' instead." .format(self.__class__.__name__, self.name, @@ -994,7 +994,7 @@ def _resolve(self, siblings): def find(self, dotted_key): if dotted_key != '': raise ValueError( - "Invalid key '{0}' for {1} called '{2}'. Since {1} don't have" + "Invalid key '{0}' for {1} called '{2}'. Since {1} don't have " "sub-elements, the key must be '' by this point." .format(dotted_key, self.__class__.__name__, self.name)) return self diff --git a/test/tests/style_tests.py b/test/tests/style_tests.py index d35a2bfe6..61110b745 100644 --- a/test/tests/style_tests.py +++ b/test/tests/style_tests.py @@ -104,12 +104,11 @@ def test_debug_prints(self): excludes=['style_tests.py', 'blarg.py', 'poof.py']) yaml_config_matches = self.find_prints( self.PAV_ROOT_DIR/'lib'/'yaml_config') + test_ex_matches = self.find_prints(self.PAV_ROOT_DIR/'lib'/'unittest_ex') - for tmatch, values in test_matches.items(): - matches[tmatch].extend(values) - - for yc_match, values in yaml_config_matches.items(): - matches[yc_match].extend(values) + for match_set in test_matches, yaml_config_matches, test_ex_matches: + for match, values in match_set.items(): + matches[match].extend(values) if matches: msg = io.StringIO() @@ -129,6 +128,73 @@ def test_debug_prints(self): PRINT_RE = re.compile(r'^\s*[^"\'#].*[^f]print\(') + def test_string_continuation(self): + """Find instances where strings continue across lines without a space + between words.""" + + bad_continuations = defaultdict(lambda: list()) + + count = 0 + for search_path in ( + self.PAV_ROOT_DIR/'lib'/'pavilion', + self.PAV_ROOT_DIR/'lib'/'yaml_config', + self.PAV_ROOT_DIR/'lib'/'unittest_ex'): + for path, _, filenames in os.walk(search_path): + for fn in filenames: + if not fn.endswith('.py'): + continue + + fn = "{}/{}".format(path, fn) + with open(fn) as file: + lines = file.readlines() + for i in range(len(lines)): + line = lines[i].strip() + + # Skip short lines and the last line. + if len(line) < 2 or i+1 >= len(lines): + continue + + next_line = lines[i+1].strip() + if not next_line: + continue + + if '"hello"' in line: + print(line) + print(next_line) + + if (line[-1] in ('"', "'") and + next_line.lstrip()[0] in ('"', "'")): + + last = line[-2] + first = next_line[1] + last_two = line[-3:-1] + first_two = next_line[1:3] + + if ( last in ('-', ' ', '/', '(') or + first in (' ', '-', '/', ')') or + first_two == '\\n' or + last_two == '\\n' or + line.endswith('"""') or + line.endswith("'''") or + next_line.startswith('"""') or + next_line.startswith("'''") + ): + continue + count += 1 + + print("first two", first_two) + + bad_continuations[fn].append((i, line, next_line)) + if bad_continuations: + msg = [] + msg.append("\nFound wrapped lines with bad spacing:") + for path, items in bad_continuations.items(): + msg.append("=> {}".format(path)) + for line_num, first, second in items: + msg.append(" {}: ... {} -> {} ...".format(line_num+1, first[-20:], second[:20])) + + self.fail('\n'.join(msg)) + def find_prints(self, path, excludes=None): """Find any code with print( statements."""