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.

Sign up for GitHub

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

Refactor pytest plugin. #76

Merged
merged 3 commits into from
Dec 15, 2015
Merged

Refactor pytest plugin. #76

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3dcd24b
Refactor pytest plugin.
Jeff-Meadows Nov 8, 2015
8112457
Merge branch 'master' into rerun_setup
Jeff-Meadows Dec 9, 2015
a71c672
Merge branch 'master' into rerun_setup
Jeff-Meadows Dec 15, 2015
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
4 changes: 2 additions & 2 deletions .pylintrc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const-rgx=(([A-Z_][A-Z0-9_]*)|(__.*__))$
class-rgx=[A-Z_][a-zA-Z0-9]+$

# Regular expression which should only match correct function names
function-rgx=[a-z_][a-z0-9_]{2,50}$
function-rgx=[a-z_][a-z0-9_]{2,60}$

# Regular expression which should only match correct method names
method-rgx=[a-z_][a-z0-9_]{2,60}$
Expand Down Expand Up @@ -124,7 +124,7 @@ docstring-min-length=-1
[FORMAT]

# Maximum number of characters on a single line.
max-line-length=80
max-line-length=120

# Regexp for a line that is allowed to be longer than the limit.
ignore-long-lines=^\s*(# )?<?https?://\S+>?$
Expand Down
127 changes: 66 additions & 61 deletions flaky/_flaky_plugin.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,16 @@ def _log_intermediate_failure(self, err, flaky, name):
)
self._log_test_failure(name, err, message)

def _should_handle_test_error_or_failure(self, test, name, err):
Copy link
Contributor

Choose a reason for hiding this comment

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

docstring. And maybe a namedtuple return type for clarity?

Leave a note in the docstring about this function needing to be pure (not setting any state)

if not self._has_flaky_attributes(test):
return False, False
flaky = self._get_flaky_attributes(test)
flaky[FlakyNames.CURRENT_RUNS] += 1
has_failed = self._has_flaky_test_failed(flaky)
if not has_failed:
return True, self._should_rerun_test(test, name, err)
return False, False

def _handle_test_error_or_failure(self, test, err):
"""
Handle a flaky test error or failure.
Expand Down Expand Up @@ -127,33 +137,31 @@ def _handle_test_error_or_failure(self, test, err):
_, _, name = self._get_test_declaration_callable_and_name(test)
except AttributeError:
return False
current_runs = self._get_flaky_attribute(
test,
FlakyNames.CURRENT_RUNS,
)
if current_runs is None:
return False
current_runs += 1
self._set_flaky_attribute(

need_reruns, should_rerun = self._should_handle_test_error_or_failure(
test,
FlakyNames.CURRENT_RUNS,
current_runs,
name,
err,
)
self._add_flaky_test_failure(test, err)
flaky = self._get_flaky_attributes(test)

if not self._has_flaky_test_failed(flaky):
if self._should_rerun_test(test, name, err):
self._log_intermediate_failure(err, flaky, name)
self._rerun_test(test)
return True
if self._has_flaky_attributes(test):
self._add_flaky_test_failure(test, err)
flaky = self._get_flaky_attributes(test)
Copy link
Contributor

Choose a reason for hiding this comment

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

Possible refactor: just use _get_flaky_attributes instead of also using _has_flaky_attributes, similar to how we use getattr(, , None) sometimes instead of hasattr()

runs = flaky[FlakyNames.CURRENT_RUNS] + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave a comment about how this is different than the one in _should_handle_test_error_or_failure (because we actually persist this one).

self._set_flaky_attribute(test, FlakyNames.CURRENT_RUNS, runs)
if need_reruns:
flaky = self._get_flaky_attributes(test)
if should_rerun:
self._log_intermediate_failure(err, flaky, name)
self._rerun_test(test)
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to _mark_test_for_rerun.

return True
else:
message = self._not_rerun_message
self._log_test_failure(name, err, message)
return False
else:
message = self._not_rerun_message
self._log_test_failure(name, err, message)
return False
else:
self._report_final_failure(err, flaky, name)
return False
self._report_final_failure(err, flaky, name)
return False

def _should_rerun_test(self, test, name, err):
"""
Expand Down Expand Up @@ -194,6 +202,14 @@ def _rerun_test(self, test):
"""
raise NotImplementedError # pragma: no cover

def _should_handle_test_success(self, test):
if not self._has_flaky_attributes(test):
return False
flaky = self._get_flaky_attributes(test)
flaky[FlakyNames.CURRENT_PASSES] += 1
flaky[FlakyNames.CURRENT_RUNS] += 1
return not self._has_flaky_test_succeeded(flaky)

def _handle_test_success(self, test):
"""
Handle a flaky test success.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't leave a line comment higher than this, but should remove nose references from the docstring.

Expand All @@ -212,48 +228,37 @@ def _handle_test_success(self, test):
:rtype:
`bool`
"""
_, _, name = self._get_test_declaration_callable_and_name(test)
current_runs = self._get_flaky_attribute(
test,
FlakyNames.CURRENT_RUNS
)
if current_runs is None:
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

Leave a comment about why this try/except is here.

_, _, name = self._get_test_declaration_callable_and_name(test)
except AttributeError:
return False
current_runs += 1
current_passes = self._get_flaky_attribute(
test,
FlakyNames.CURRENT_PASSES
)
current_passes += 1
self._set_flaky_attribute(
test,
FlakyNames.CURRENT_RUNS,
current_runs
)
self._set_flaky_attribute(
test,
FlakyNames.CURRENT_PASSES,
current_passes
)
flaky = self._get_flaky_attributes(test)
need_reruns = not self._has_flaky_test_succeeded(flaky)
if self._flaky_success_report:
need_reruns = self._should_handle_test_success(test)

if self._has_flaky_attributes(test):
flaky = self._get_flaky_attributes(test)
min_passes = flaky[FlakyNames.MIN_PASSES]
self._stream.writelines([
ensure_unicode_string(name),
' passed {0} out of the required {1} times. '.format(
current_passes,
min_passes,
),
])
if need_reruns:
self._stream.write(
'Running test again until it passes {0} times.\n'.format(
passes = flaky[FlakyNames.CURRENT_PASSES] + 1
runs = flaky[FlakyNames.CURRENT_RUNS] + 1
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have helper methods for:

  • Changing flaky attributes to increase current_runs.
  • Changing flaky attributes to increase current_runs and current_passes.

Then we don't need to duplicate code.

self._set_flaky_attribute(test, FlakyNames.CURRENT_PASSES, passes)
self._set_flaky_attribute(test, FlakyNames.CURRENT_RUNS, runs)

if self._flaky_success_report:
self._stream.writelines([
ensure_unicode_string(name),
' passed {0} out of the required {1} times. '.format(
passes,
min_passes,
),
])
if need_reruns:
self._stream.write(
'Running test again until it passes {0} times.\n'.format(
min_passes,
)
)
)
else:
self._stream.write('Success!\n')
else:
self._stream.write('Success!\n')

if need_reruns:
self._rerun_test(test)
return need_reruns
Expand Down
Loading