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

Print level feature #33

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
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
46 changes: 23 additions & 23 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ for files. In this case, you can use urlchecker check:
```bash
$ urlchecker check --help
usage: urlchecker check [-h] [-b BRANCH] [--subfolder SUBFOLDER] [--cleanup]
[--force-pass] [--no-print] [--file-types FILE_TYPES]
[--files FILES]
[--force-pass] [--file-types FILE_TYPES]
[--files FILES] [--print-level PRINT_LEVEL]
[--white-listed-urls WHITE_LISTED_URLS]
[--white-listed-patterns WHITE_LISTED_PATTERNS]
[--white-listed-files WHITE_LISTED_FILES]
Expand All @@ -72,13 +72,14 @@ optional arguments:
no cleaup)
--force-pass force successful pass (return code 0) regardless of
result
--no-print Skip printing results to the screen (defaults to
printing to console).
--file-types FILE_TYPES
comma separated list of file extensions to check
(defaults to .md,.py)
--files FILES comma separated list of exact files or patterns to
check.
--print-level PRINT_LEVEL
print levels: all, files-with-urls-only, fails-only,
success-only, none
--white-listed-urls WHITE_LISTED_URLS
comma separated list of white listed urls (no spaces)
--white-listed-patterns WHITE_LISTED_PATTERNS
Expand Down Expand Up @@ -111,7 +112,7 @@ $ urlchecker check .
branch: master
cleanup: False
file types: ['.md', '.py']
print all: True
print level: all
url whitetlist: []
url patterns: []
file patterns: []
Expand Down Expand Up @@ -188,7 +189,7 @@ $ urlchecker check --save results.csv .
branch: master
cleanup: False
file types: ['.md', '.py']
print all: True
print level: all
url whitetlist: []
url patterns: []
file patterns: []
Expand All @@ -197,18 +198,18 @@ $ urlchecker check --save results.csv .
save: results.csv
timeout: 5

/home/vanessa/Desktop/Code/urlstechie/urlchecker-test-repo/README.md
/home/vanessa/Desktop/Code/urlstechie/urlchecker-test-repo/README.md
--------------------------------------------------------------------
No urls found.

/home/vanessa/Desktop/Code/urlstechie/urlchecker-test-repo/test_files/sample_test_file.py
/home/vanessa/Desktop/Code/urlstechie/urlchecker-test-repo/test_files/sample_test_file.py
-----------------------------------------------------------------------------------------
https://github.com/SuperKogito/URLs-checker/README.md
https://github.com/SuperKogito/URLs-checker/README.md
https://www.google.com/
https://github.com/SuperKogito

/home/vanessa/Desktop/Code/urlstechie/urlchecker-test-repo/test_files/sample_test_file.md
/home/vanessa/Desktop/Code/urlstechie/urlchecker-test-repo/test_files/sample_test_file.md
-----------------------------------------------------------------------------------------
https://github.com/SuperKogito/URLs-checker/blob/master/README.md
https://github.com/SuperKogito/Voice-based-gender-recognition/issues
Expand Down Expand Up @@ -296,7 +297,7 @@ checker = UrlChecker(
file_types=[".md", ".py", ".rst"],
include_patterns=[],
white_listed_files=["README.md", "LICENSE"],
print_all=True,
print_level="all",
)
```
I can then run the checker like this:
Expand Down Expand Up @@ -349,32 +350,32 @@ You can look at `checker.checks`, which is a dictionary of result objects,
organized by the filename:

```python
for file_name, result in checker.checks.items():
print()
print(result)
print("Total Results: %s " % result.count)
print("Total Failed: %s" % len(result.failed))
print("Total Passed: %s" % len(result.passed))
for file_name, result in checker.checks.items():
print()
print(result)
print("Total Results: %s " % result.count)
print("Total Failed: %s" % len(result.failed))
print("Total Passed: %s" % len(result.passed))

...

UrlCheck:/home/vanessa/Desktop/Code/urlstechie/urlchecker-python/tests/test_files/sample_test_file.md
Total Results: 26
Total Results: 26
Total Failed: 6
Total Passed: 20

UrlCheck:/home/vanessa/Desktop/Code/urlstechie/urlchecker-python/.pytest_cache/README.md
Total Results: 1
Total Results: 1
Total Failed: 0
Total Passed: 1

UrlCheck:/home/vanessa/Desktop/Code/urlstechie/urlchecker-python/.eggs/pytest_runner-5.2-py3.7.egg/ptr.py
Total Results: 0
Total Results: 0
Total Failed: 0
Total Passed: 0

UrlCheck:/home/vanessa/Desktop/Code/urlstechie/urlchecker-python/docs/source/conf.py
Total Results: 3
Total Results: 3
Total Failed: 0
Total Passed: 3
```
Expand Down Expand Up @@ -425,7 +426,7 @@ checker = UrlCheckResult(
file_name=file_name,
white_listed_patterns=white_listed_patterns,
white_listed_urls=white_listed_urls,
print_all=self.print_all,
print_level=self.print_level,
)
```

Expand All @@ -435,7 +436,7 @@ or you can provide all the parameters without the filename:
checker = UrlCheckResult(
white_listed_patterns=white_listed_patterns,
white_listed_urls=white_listed_urls,
print_all=self.print_all,
print_level=self.print_level,
)
```

Expand Down Expand Up @@ -522,4 +523,3 @@ be fairly straight forward to always find what you are looking for.

If you need help, or want to suggest a project for the organization,
please [open an issue](https://github.com/urlstechie/urlchecker-python)

9 changes: 5 additions & 4 deletions tests/test_client_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

@pytest.mark.parametrize("config_fname", ["./tests/_local_test_config.conf"])
@pytest.mark.parametrize("cleanup", [False, True])
@pytest.mark.parametrize("print_all", [False, True])
@pytest.mark.parametrize("print_level", ["all", "files-with-urls-only", "fails-only", "success-only", "none"])
@pytest.mark.parametrize("force_pass", [False, True])
@pytest.mark.parametrize("rcount", [1, 3])
@pytest.mark.parametrize("timeout", [3, 5])
def test_client_general(config_fname, cleanup, print_all, force_pass, rcount, timeout):
def test_client_general(config_fname, cleanup, print_level, force_pass, rcount, timeout):

# init config parser
config = configparser.ConfigParser()
Expand All @@ -33,6 +33,8 @@ def test_client_general(config_fname, cleanup, print_all, force_pass, rcount, ti
file_types,
"--white-listed-files",
"conf.py",
"--print_level",
print_level,
"--white-listed-urls",
white_listed_urls,
"--white-listed_patterns",
Expand All @@ -46,8 +48,7 @@ def test_client_general(config_fname, cleanup, print_all, force_pass, rcount, ti
# Add boolean arguments
if cleanup:
cmd.append("--cleanup")
if print_all:
cmd.append("--print-all")

if force_pass:
cmd.append("--force-pass")

Expand Down
15 changes: 8 additions & 7 deletions tests/test_core_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,22 @@
["tests/test_files/sample_test_file.rst"],
],
)
@pytest.mark.parametrize("print_all", [False, True])
@pytest.mark.parametrize(
"print_level", ["all", "files-with-urls-only",
"fails-only", "success-only", "none"]
)
@pytest.mark.parametrize(
"white_listed_urls", [["https://github.com/SuperKogito/SuperKogito.github.io"]]
)
@pytest.mark.parametrize(
"white_listed_patterns",
[[], ["https://github.com/SuperKogito/SuperKogito.github.io"]],
)
def test_check_files(file_paths, print_all, white_listed_urls, white_listed_patterns):
def test_check_files(file_paths, print_level, white_listed_urls, white_listed_patterns):
"""
test check repo function.
"""
checker = UrlChecker(print_all=print_all)
checker = UrlChecker(print_level=print_level)
checker.run(
file_paths,
white_listed_urls=white_listed_urls,
Expand All @@ -46,15 +49,14 @@ def test_locally(local_folder_path, config_fname):
# read input variables
git_path = local_folder_path
file_types = config["DEFAULT"]["file_types_test_values"].split(",")
print_all = True
white_listed_urls = config["DEFAULT"]["white_listed_test_urls"].split(",")
white_listed_patterns = config["DEFAULT"]["white_listed__test_patterns"].split(",")

# get all file paths
file_paths = get_file_paths(git_path, file_types)

# check repo urls
checker = UrlChecker(print_all=print_all)
checker = UrlChecker(print_level="all")
checker.run(
file_paths=file_paths,
white_listed_urls=white_listed_urls,
Expand All @@ -69,7 +71,6 @@ def test_check_run_save(tmp_path, retry_count):
# init vars
git_path = "https://github.com/urlstechie/urlchecker-test-repo"
file_types = [".py", ".md"]
print_all = True
white_listed_urls = [
"https://superkogito.github.io/figures/fig2.html",
"https://superkogito.github.io/figures/fig4.html",
Expand All @@ -86,7 +87,7 @@ def test_check_run_save(tmp_path, retry_count):
file_paths = get_file_paths(base_path, file_types)

# check repo urls
checker = UrlChecker(print_all=print_all)
checker = UrlChecker(print_level="all")
check_results = checker.run(
file_paths=file_paths,
white_listed_urls=white_listed_urls,
Expand Down
14 changes: 8 additions & 6 deletions tests/test_core_urlproc.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ def test_check_urls(file):
# Ensure one not whitelisted is failed
assert "https://github.com/SuperKogito/URLs-checker/issues/1" in checker.failed

assert checker.print_all

# Run again with whitelist of exact urls
checker = UrlCheckResult(
white_listed_urls=["https://github.com/SuperKogito/URLs-checker/issues/1"]
Expand All @@ -61,7 +59,11 @@ def test_get_user_agent():
assert isinstance(user_agent, str)


def test_check_response_status_code():
@pytest.mark.parametrize(
"print_level", ["all", "files-with-urls-only",
Copy link
Collaborator

Choose a reason for hiding this comment

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

How is files-with-urls-only honored? I don't see it in the printing function, only the other four.

Copy link
Member Author

Choose a reason for hiding this comment

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

so files-with-urls-only is not explicitly specified in the code, because I did not need to. Covering the other cases was sufficient. So -as you already noticed- you barely see the variable in the code, but if used, it works as expected.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn’t make any sense. Please provide an explicit example. If an option isn’t used I don’t understand why it exists.

"fails-only", "success-only", "none"]
)
def test_check_response_status_code(print_level):
class failedResponse:
status_code = 500

Expand All @@ -70,7 +72,7 @@ class successResponse:

# Any failure returns True (indicating a retry is needed)
assert not check_response_status_code(
"https://this-should-succeed", successResponse
"https://this-should-succeed", successResponse, print_level
)
assert check_response_status_code("https://this-should-fail", failedResponse)
assert check_response_status_code("https://this-should-also-fail", None)
assert check_response_status_code("https://this-should-fail", failedResponse, print_level)
assert check_response_status_code("https://this-should-also-fail", None, print_level)
20 changes: 12 additions & 8 deletions urlchecker/client/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,13 +85,6 @@ def get_parser():
action="store_true",
)

check.add_argument(
"--no-print",
help="Skip printing results to the screen (defaults to printing to console).",
default=False,
action="store_true",
)

check.add_argument(
"--file-types",
dest="file_types",
Expand All @@ -106,6 +99,17 @@ def get_parser():
default="",
)

# Printing

check.add_argument(
"--print-level",
dest="print_level",
help="print levels: all, files-with-urls-only, fails-only, success-only, none",
choices=["all", "files-with-urls-only", "fails-only", "success-only", "none"],
default="all",
)


# White listing

check.add_argument(
Expand Down Expand Up @@ -158,7 +162,7 @@ def main():
parser = get_parser()

def help(return_code=0):
"""print help, including the software version and active client
"""print help, including the software version and active client
and exit with return code.
"""

Expand Down
4 changes: 2 additions & 2 deletions urlchecker/client/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def main(args, extra):
print(" cleanup: %s" % args.cleanup)
print(" file types: %s" % file_types)
print(" files: %s" % files)
print(" print all: %s" % (not args.no_print))
print(" print_level: %s" % args.print_level)
print(" url whitetlist: %s" % white_listed_urls)
print(" url patterns: %s" % white_listed_patterns)
print(" file patterns: %s" % white_listed_files)
Expand All @@ -77,7 +77,7 @@ def main(args, extra):
file_types=file_types,
include_patterns=files,
white_listed_files=white_listed_files,
print_all=not args.no_print,
print_level=args.print_level,
)
check_results = checker.run(
white_listed_urls=white_listed_urls,
Expand Down
8 changes: 4 additions & 4 deletions urlchecker/core/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def __init__(
path=None,
file_types=None,
white_listed_files=None,
print_all=True,
print_level=None,
include_patterns=None,
):
"""
Expand All @@ -34,7 +34,7 @@ def __init__(

Args:
- path (str) : full path to the root folder to check. If not defined, no file_paths are parsed
- print_all (str) : control var for whether to print all checked file names or only the ones with urls.
- print_level (str) : control the printing level.
- white_listed_files (list) : list of white-listed files and patterns for flies.
- include_patterns (list) : list of files and patterns to check.
"""
Expand All @@ -45,7 +45,7 @@ def __init__(
# Save run parameters
self.white_listed_files = white_listed_files or []
self.include_patterns = include_patterns or []
self.print_all = print_all
self.print_level = print_level
Copy link
Collaborator

Choose a reason for hiding this comment

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

For all the places that print_level is set from a client, we have to assume that a user might set it to None, thinking this is default or "unset." So instead of just blindly setting it like this, let's do something like:

self.print_level = print_level or "none"

That way if it's set to an empty string or None, it will not break the client.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But since we favor verbosity, the default should be:

self.print_level = print_level or "all"

Copy link
Member Author

Choose a reason for hiding this comment

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

makes sense. I will make the change.

self.path = path
self.file_types = file_types or [".py", ".md"]
self.file_paths = []
Expand Down Expand Up @@ -155,7 +155,7 @@ def run(
file_name=file_name,
white_listed_patterns=white_listed_patterns,
white_listed_urls=white_listed_urls,
print_all=self.print_all,
print_level=self.print_level,
)

# Check the urls
Expand Down
Loading