Skip to content

Commit

Permalink
Update progress after iteration (fix #651)
Browse files Browse the repository at this point in the history
Reported "off by one" error is due to the fact that an interator is used
instead of a generator. As such there is no way for the bar to know when
it has iterated over the user's code block.

This commit swaps the iterator for a generator, and changes _when_ the
status report is updated, from before each iteration of a user's code
block, to _after_.

Updating the status report after the user's code has run allows the
report to convey how much work has been _done_ so far.

Accidental gain:
The associated unit test performs a black-box test on the generated
progress output. This test revealed a bug in the ETA time calculation
were the behavior differed between python 2 and 3, causing the unit test
to fail on python 3. The remedy employed to allow passing of the test is
to use integer division for the ETA calculation, which should also
address a related issue (fixes #485).
  • Loading branch information
jojje committed Dec 22, 2016
1 parent e6f22c6 commit 8780a76
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 19 deletions.
40 changes: 21 additions & 19 deletions click/_termui_impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def __iter__(self):
if not self.entered:
raise RuntimeError('You need to use progress bars in a with block.')
self.render_progress()
return self
return self.generator()

def render_finish(self):
if self.is_hidden:
Expand Down Expand Up @@ -130,13 +130,13 @@ def eta(self):

def format_eta(self):
if self.eta_known:
t = self.eta + 1
t = int(self.eta)
seconds = t % 60
t /= 60
t //= 60
minutes = t % 60
t /= 60
t //= 60
hours = t % 24
t /= 24
t //= 24
if t > 0:
days = t
return '%dd %02d:%02d:%02d' % (days, hours, minutes, seconds)
Expand Down Expand Up @@ -253,23 +253,25 @@ def finish(self):
self.current_item = None
self.finished = True

def next(self):
def generator(self):
"""
Returns a generator which yields the items added to the bar during
construction, and updates the progress bar *after* the yielded block
returns.
"""
if not self.entered:
raise RuntimeError('You need to use progress bars in a with block.')

if self.is_hidden:
return next(self.iter)
try:
rv = next(self.iter)
self.current_item = rv
except StopIteration:
for rv in self.iter:
yield rv
else:
for rv in self.iter:
self.current_item = rv
yield rv
self.update(1)
self.finish()
self.render_progress()
raise StopIteration()
else:
self.update(1)
return rv

if not PY2:
__next__ = next
del next


def pager(text, color=None):
Expand Down
39 changes: 39 additions & 0 deletions tests/test_termui.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import click
import time


def test_progressbar_strip_regression(runner, monkeypatch):
Expand Down Expand Up @@ -69,3 +70,41 @@ def test_secho(runner):
click.secho(None, nl=False)
bytes = out.getvalue()
assert bytes == b''


def test_progressbar_yields_all_items(runner):
with click.progressbar(range(3)) as progress:
assert len(list(progress)) == 3


def test_progressbar_update(runner, monkeypatch):
class FakeClock(object):
def __init__(self):
self.now = time.time()

def advance_time(self, seconds=1):
self.now += seconds

def time(self):
return self.now

fake_clock = FakeClock()

@click.command()
def cli():
with click.progressbar(range(4)) as progress:
for _ in progress:
fake_clock.advance_time()
print("")

monkeypatch.setattr(time, 'time', fake_clock.time)
monkeypatch.setattr(click._termui_impl, 'isatty', lambda _: True)
output = runner.invoke(cli, []).output

lines = [line for line in output.split('\n') if '[' in line]

assert ' 0%' in lines[0]
assert ' 25% 00:00:03' in lines[1]
assert ' 50% 00:00:02' in lines[2]
assert ' 75% 00:00:01' in lines[3]
assert '100% ' in lines[4]

0 comments on commit 8780a76

Please sign in to comment.