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

@@ -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,

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 = [

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

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.",

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.")
 

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"

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'."
         )
 

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(

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
 

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=''

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."

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 '
                  '<suite_name>.<test_name>.')
 
     SLEEP_INTERVAL = 1

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,16 +381,15 @@ 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(
             "log_format",
             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:

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

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))
 

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:

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)))

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:"

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()