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

make flake8 code style checks pass in test/ #3416

Merged
merged 4 commits into from
Aug 20, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 .github/workflows/linting.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,4 @@ jobs:
- name: Run flake8
run: |
flake8 easybuild/tools
flake8 test/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not change this during the sprint. My suggestion is to enable this at the very end for .. Otherwise all other PRs will conflict when changing this too.
Oh and according to GH it is missing the final newline and the trailing slash is not required :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general, I agree.

But in this case, it's fine to leave this in if @wpoely86 bases his work for the easybuild folder on top of this PR (all the changes there can be made in one go, and then linting.yml can be changed to use flake8 ., and we're done?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure if you want to make those dependent PRs but IMO it is easier to verify locally via flake8 and don't change this yet.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For easyblocks repo, I definitely agree we should hold off touching (actually adding) linting.yml until we're done.

6 changes: 3 additions & 3 deletions test/framework/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -190,8 +190,8 @@ def test_error_env_var_typo(self):
os.environ['EASYBUILD_THERESNOSUCHCONFIGURATIONOPTION'] = 'whatever'

error = r"Found 2 environment variable\(s\) that are prefixed with %s " % CONFIG_ENV_VAR_PREFIX
error += "but do not match valid option\(s\): "
error += ','.join(['EASYBUILD_FOO', 'EASYBUILD_THERESNOSUCHCONFIGURATIONOPTION'])
error += r"but do not match valid option\(s\): "
error += r','.join(['EASYBUILD_FOO', 'EASYBUILD_THERESNOSUCHCONFIGURATIONOPTION'])
self.assertErrorRegex(EasyBuildError, error, init_config)

del os.environ['EASYBUILD_THERESNOSUCHCONFIGURATIONOPTION']
Expand Down Expand Up @@ -365,7 +365,7 @@ def test_build_options(self):

# only valid keys can be set
BuildOptions.__class__._instances.clear()
msg = "Encountered unknown keys .* \(known keys: .*"
msg = r"Encountered unknown keys .* \(known keys: .*"
self.assertErrorRegex(KeyError, msg, BuildOptions, {'thisisclearlynotavalidbuildoption': 'FAIL'})

# test init_build_options and build_option functions
Expand Down
10 changes: 5 additions & 5 deletions test/framework/containers.py
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ def test_end2end_singularity_image(self):
args.append('--container-tmpdir=%s' % self.test_prefix)
stdout, stderr = self.run_main(args)
self.assertFalse(stderr)
regexs[-3] = "^== Running 'sudo\s*SINGULARITY_TMPDIR=%s \S*/singularity build .*" % self.test_prefix
regexs[-3] = r"^== Running 'sudo\s*SINGULARITY_TMPDIR=%s \S*/singularity build .*" % self.test_prefix
self.check_regexs(regexs, stdout)

def test_end2end_dockerfile(self):
Expand Down Expand Up @@ -459,10 +459,10 @@ def test_end2end_docker_image(self):
stdout, stderr = self.run_main(args)
self.assertFalse(stderr)
regexs = [
"^== docker tool found at %s/bin/docker" % self.test_prefix,
"^== Dockerfile definition file created at %s/containers/Dockerfile\.toy-0.0" % self.test_prefix,
"^== Running 'sudo docker build -f .* -t .* \.', you may need to enter your 'sudo' password...",
"^== Docker image created at toy-0.0:latest",
r"^== docker tool found at %s/bin/docker" % self.test_prefix,
r"^== Dockerfile definition file created at %s/containers/Dockerfile\.toy-0.0" % self.test_prefix,
r"^== Running 'sudo docker build -f .* -t .* \.', you may need to enter your 'sudo' password...",
r"^== Docker image created at toy-0.0:latest",
]
self.check_regexs(regexs, stdout)

Expand Down
2 changes: 1 addition & 1 deletion test/framework/docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def test_license_docs(self):
self.assertTrue(gplv3 in lic_docs, "%s found in: %s" % (gplv3, lic_docs))

lic_docs = avail_easyconfig_licenses(output_format='rst')
regex = re.compile("^``GPLv3``\s*The GNU General Public License", re.M)
regex = re.compile(r"^``GPLv3``\s*The GNU General Public License", re.M)
self.assertTrue(regex.search(lic_docs), "%s found in: %s" % (regex.pattern, lic_docs))

def test_list_software(self):
Expand Down
6 changes: 3 additions & 3 deletions test/framework/easyblock.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ def test_make_module_extend_modpath(self):
elif get_module_syntax() == 'Lua':
regexs = [r'^prepend_path\("MODULEPATH", ".*/modules/funky/Compiler/pi/3.14/%s"\)$' % c for c in modclasses]
home = r'os.getenv\("HOME"\)'
pj_usermodsdir = 'pathJoin\("%s", "funky", "Compiler/pi/3.14"\)' % usermodsdir
pj_usermodsdir = r'pathJoin\("%s", "funky", "Compiler/pi/3.14"\)' % usermodsdir
regexs.extend([
# extension for user modules is guarded
r'if isDir\(pathJoin\(%s, %s\)\) then' % (home, pj_usermodsdir),
Expand Down Expand Up @@ -1061,7 +1061,7 @@ def test_make_module_step(self):

# [==[ or ]==] in description is fatal
if get_module_syntax() == 'Lua':
error_pattern = "Found unwanted '\[==\[' or '\]==\]' in: .*"
error_pattern = r"Found unwanted '\[==\[' or '\]==\]' in: .*"
for descr in ["test [==[", "]==] foo"]:
ectxt = read_file(self.eb_file)
write_file(self.eb_file, re.sub('description.*', 'description = "%s"' % descr, ectxt))
Expand Down Expand Up @@ -1263,7 +1263,7 @@ def test_fetch_sources(self):

# old format for specifying source with custom extract command is deprecated
eb.src = []
error_msg = "DEPRECATED \(since v4.0\).*Using a 2-element list/tuple.*"
error_msg = r"DEPRECATED \(since v4.0\).*Using a 2-element list/tuple.*"
self.assertErrorRegex(EasyBuildError, error_msg, eb.fetch_sources,
[('toy-0.0_gzip.patch.gz', "gunzip %s")], checksums=[])

Expand Down
12 changes: 6 additions & 6 deletions test/framework/easyconfigversion.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,10 +114,10 @@ def test_versop_overlap_conflict(self):
('> 2 suffix:-x1', '< 1 suffix:-x1', (False, False)), # suffix equal, no conflict (and no overlap)
]

for l, r, res in overlap_conflict:
vl = VersionOperator(l)
vr = VersionOperator(r)
self.assertEqual(vl.test_overlap_and_conflict(vr), res)
for left, right, res in overlap_conflict:
verop_left = VersionOperator(left)
verop_right = VersionOperator(right)
self.assertEqual(verop_left.test_overlap_and_conflict(verop_right), res)

def test_versop_gt(self):
"""Test strict greater then ordering"""
Expand All @@ -135,8 +135,8 @@ def test_versop_gt(self):
# suffix
('> 2 suffix:-x1', '> 1 suffix:-x1'), # equal suffixes, regular ordering
]
for l, r in left_gt_right:
self.assertTrue(VersionOperator(l) > VersionOperator(r), "%s gt %s" % (l, r))
for left, right in left_gt_right:
self.assertTrue(VersionOperator(left) > VersionOperator(right), "%s gt %s" % (left, right))

def test_ordered_versop_expressions(self):
"""Given set of ranges, order them according to version/operator (most recent/specific first)"""
Expand Down
7 changes: 3 additions & 4 deletions test/framework/filetools.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ def test_find_glob_pattern(self):
self.assertErrorRegex(EasyBuildError, "Was expecting exactly", ft.find_glob_pattern,
os.path.join(tmpdir, 'python*'))


def test_encode_class_name(self):
"""Test encoding of class names."""
for (class_name, encoded_class_name) in self.class_names:
Expand Down Expand Up @@ -929,7 +928,7 @@ def test_multidiff(self):
expected = "29 %s+ postinstallcmds = " % green
self.assertTrue(any([line.startswith(expected) for line in lines]))
expected = "30 %s+%s (1/2) toy-0.0" % (green, endcol)
self.assertTrue(any(l.startswith(expected) for l in lines), "Found '%s' in: %s" % (expected, lines))
self.assertTrue(any(line.startswith(expected) for line in lines), "Found '%s' in: %s" % (expected, lines))
self.assertEqual(lines[-1], "=====")

lines = multidiff(toy_ec, other_toy_ecs, colored=False).split('\n')
Expand All @@ -948,9 +947,9 @@ def test_multidiff(self):

# no postinstallcmds in toy-0.0-deps.eb
expected = "29 + postinstallcmds = "
self.assertTrue(any(l.startswith(expected) for l in lines), "Found '%s' in: %s" % (expected, lines))
self.assertTrue(any(line.startswith(expected) for line in lines), "Found '%s' in: %s" % (expected, lines))
expected = "30 + (1/2) toy-0.0-"
self.assertTrue(any(l.startswith(expected) for l in lines), "Found '%s' in: %s" % (expected, lines))
self.assertTrue(any(line.startswith(expected) for line in lines), "Found '%s' in: %s" % (expected, lines))

self.assertEqual(lines[-1], "=====")

Expand Down
6 changes: 3 additions & 3 deletions test/framework/general.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ def test_error_reporting(self):
easybuild_loc = os.path.dirname(os.path.dirname(os.path.abspath(easybuild.framework.__file__)))

log_method_regexes = [
re.compile("log\.error\("),
re.compile("log\.exception\("),
re.compile("log\.raiseException\("),
re.compile(r"log\.error\("),
re.compile(r"log\.exception\("),
re.compile(r"log\.raiseException\("),
]

for dirpath, _, filenames in os.walk(easybuild_loc):
Expand Down
2 changes: 1 addition & 1 deletion test/framework/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ def test_find_easybuild_easyconfig(self):
print("Skipping test_find_easybuild_easyconfig, no GitHub token available?")
return
path = gh.find_easybuild_easyconfig(github_user=GITHUB_TEST_ACCOUNT)
expected = os.path.join('e', 'EasyBuild', 'EasyBuild-[1-9]+\.[0-9]+\.[0-9]+\.eb')
expected = os.path.join('e', 'EasyBuild', r'EasyBuild-[1-9]+\.[0-9]+\.[0-9]+\.eb')
regex = re.compile(expected)
self.assertTrue(regex.search(path), "Pattern '%s' found in '%s'" % (regex.pattern, path))
self.assertTrue(os.path.exists(path), "Path %s exists" % path)
Expand Down
12 changes: 6 additions & 6 deletions test/framework/lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ def configure(self):
def test_run_cmd(self):
"""Test use of run_cmd function in the context of using EasyBuild framework as a library."""

error_pattern = "Undefined build option: .*"
error_pattern += " Make sure you have set up the EasyBuild configuration using set_up_configuration\(\)"
error_pattern = r"Undefined build option: .*"
error_pattern += r" Make sure you have set up the EasyBuild configuration using set_up_configuration\(\)"
self.assertErrorRegex(EasyBuildError, error_pattern, run_cmd, "echo hello")

self.configure()
Expand All @@ -95,8 +95,8 @@ def test_mkdir(self):

test_dir = os.path.join(self.tmpdir, 'test123')

error_pattern = "Undefined build option: .*"
error_pattern += " Make sure you have set up the EasyBuild configuration using set_up_configuration\(\)"
error_pattern = r"Undefined build option: .*"
error_pattern += r" Make sure you have set up the EasyBuild configuration using set_up_configuration\(\)"
self.assertErrorRegex(EasyBuildError, error_pattern, mkdir, test_dir)

self.configure()
Expand All @@ -109,8 +109,8 @@ def test_mkdir(self):
def test_modules_tool(self):
"""Test use of modules_tool function in the context of using EasyBuild framework as a library."""

error_pattern = "Undefined build option: .*"
error_pattern += " Make sure you have set up the EasyBuild configuration using set_up_configuration\(\)"
error_pattern = r"Undefined build option: .*"
error_pattern += r" Make sure you have set up the EasyBuild configuration using set_up_configuration\(\)"
self.assertErrorRegex(EasyBuildError, error_pattern, modules_tool)

self.configure()
Expand Down
Loading