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

feat: add dry-run option #78

Merged
merged 4 commits into from
Oct 8, 2018
Merged
Changes from 3 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
19 changes: 19 additions & 0 deletions README.rst
Original file line number Diff line number Diff line change
@@ -121,6 +121,25 @@ these exact identifiers directly, if you need something specific.
The ``source`` release is always automatically included. ``pip`` will use
this as a fallback in the case a suitable wheel cannot be found.

Dry run mode
============

There are some use cases, when you maybe don't want to edit your requirements.txt
right away. You can use the ``-dry-run`` argument to just print the lines, as if they
would be added to your requirements.txt file.
max-wittig marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that this still should have backticks aroud requirements.txt. Also, the argument is listed as -dry-run with only one -, when it should two.

More holistically, didn't this change so that it shows a diff, isntead of "just print[ing] the lines"?


Example::

hashin --dry-run requests==2.19.1

Would result in a printout on the command line::

requests==2.19.1 \
--hash=sha256:63b52e3c866428a224f97cab011de738c36aec0185aa91cfacd418b5d58911d1 \
--hash=sha256:ec22d826a36ed72a7358ff3fe56cbd4ba69dd7a6718ffd450ff0e9df7a47ce6a



PEP-0496 Environment Markers
============================

16 changes: 14 additions & 2 deletions hashin.py
Original file line number Diff line number Diff line change
@@ -75,6 +75,12 @@
action='store_true',
default=False,
)
parser.add_argument(
'--dry-run',
help='Don\'t touch requirements.txt and just show hashes',
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Most style guides allows for switching quotes to avoid escaping quotes in a string.

Copy link
Owner

Choose a reason for hiding this comment

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

I wouldn't worry about this one. Once this PR is squared away there are no open PRs and I can take a massive knife of black and therapist to this code base and then these kinds of nits become unimportant.

action='store_true',
default=False,
)


major_pip_version = int(pip_api.version().split('.')[0])
@@ -101,7 +107,7 @@ def _download(url, binary=False):
# Note that urlopen will, by default, follow redirects.
status_code = r.getcode()

if status_code >= 301 and status_code < 400:
if 301 <= status_code < 400:
location, _ = cgi.parse_header(r.headers.get('location', ''))
if not location:
raise PackageError("No 'Location' header on {0} ({1})".format(
@@ -137,6 +143,7 @@ def run_single_package(
python_versions=None,
verbose=False,
include_prereleases=False,
dry_run=False,
):
restriction = None
if ';' in spec:
@@ -159,7 +166,6 @@ def run_single_package(
package = data['package']

maybe_restriction = '' if not restriction else '; {0}'.format(restriction)
new_lines = ''
new_lines = '{0}=={1}{2} \\\n'.format(
package,
data['version'],
@@ -175,6 +181,11 @@ def run_single_package(
new_lines += ' \\'
new_lines += '\n'

if dry_run:
if verbose:
_verbose('Dry run, not editing ', file)
print(new_lines)
return
if verbose:
_verbose('Editing', file)
with open(file) as f:
@@ -498,6 +509,7 @@ def main():
args.python_version,
verbose=args.verbose,
include_prereleases=args.include_prereleases,
dry_run=args.dry_run,
)
except PackageError as exception:
print(str(exception), file=sys.stderr)
4 changes: 4 additions & 0 deletions tests/test_arg_parse.py
Original file line number Diff line number Diff line change
@@ -19,6 +19,7 @@ def test_everything():
verbose=True,
version=False,
include_prereleases=False,
dry_run=False,
max-wittig marked this conversation as resolved.
Show resolved Hide resolved
)
assert args == (expected, [])

@@ -30,6 +31,7 @@ def test_everything_long():
'--algorithm', 'sha512',
'--python-version', '3.5',
'--verbose',
'--dry-run',
])
expected = argparse.Namespace(
algorithm='sha512',
@@ -39,6 +41,7 @@ def test_everything_long():
verbose=True,
version=False,
include_prereleases=False,
dry_run=True,
)
assert args == (expected, [])

@@ -53,5 +56,6 @@ def test_minimal():
verbose=False,
version=False,
include_prereleases=False,
dry_run=False,
Copy link
Owner

Choose a reason for hiding this comment

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

Isn't this the default anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests fail otherwise here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I leave it like this or what changes should be made?

)
assert args == (expected, [])
66 changes: 66 additions & 0 deletions tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -685,6 +685,72 @@ def mocked_get(url, **options):
'hashin==0.11 \\'
)

@cleanup_tmpdir('hashin*')
@mock.patch('hashin.urlopen')
def test_run_dry(self, murlopen):
"""dry run should edit the requirements.txt file and print
hashes and package name in the console
"""

def mocked_get(url, **options):
if url == 'https://pypi.org/pypi/hashin/json':
return _Response({
'info': {
'version': '0.11',
'name': 'hashin',
},
'releases': {
'0.11': [
{
'url': 'https://pypi.org/packages/source/p/hashin/hashin-0.11.tar.gz',
'digests': {
'sha256': 'bbbbb',
},
}
],
'0.10': [
{
'url': 'https://pypi.org/packages/source/p/hashin/hashin-0.10.tar.gz',
'digests': {
'sha256': 'aaaaa',
},
}
]
}
})

murlopen.side_effect = mocked_get

with tmpfile() as filename:
with open(filename, 'w') as f:
f.write('')

my_stdout = StringIO()
with redirect_stdout(my_stdout):
retcode = hashin.run(
'hashin==0.10',
filename,
'sha256',
verbose=False,
dry_run=True,
)

self.assertEqual(retcode, 0)

# verify that nothing has been written to file
with open(filename) as f:
output = f.read()
assert not output

# Check dry run output
out_lines = my_stdout.getvalue().splitlines()
self.assertTrue(
'hashin==0.10' in out_lines[0]
)
self.assertTrue(
'--hash=sha256:aaaaa' in out_lines[1]
)

@cleanup_tmpdir('hashin*')
@mock.patch('hashin.urlopen')
def test_run_pep_0496(self, murlopen):