Skip to content

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Mar 1, 2019

Moved/added there in 250160b, but apparently not intentionally?!

# noqa was only added later in two steps.

/cc @benjaminp

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

If tests pass and @benjaminp agrees, LGTM!

@codecov
Copy link

codecov bot commented Mar 1, 2019

Codecov Report

Merging #4863 into features will decrease coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##           features    #4863      +/-   ##
============================================
- Coverage     95.75%   95.65%   -0.11%     
============================================
  Files           113      113              
  Lines         25724    25216     -508     
  Branches       2511     2504       -7     
============================================
- Hits          24632    24120     -512     
- Misses          773      775       +2     
- Partials        319      321       +2
Flag Coverage Δ
#linux 95.46% <100%> (-0.12%) ⬇️
#osx 93.31% <100%> (+0.01%) ⬆️
#windows 93.58% <100%> (ø) ⬆️
Impacted Files Coverage Δ
src/_pytest/assertion/rewrite.py 95.47% <100%> (-0.02%) ⬇️
testing/test_modimport.py 81.81% <0%> (-1.52%) ⬇️
testing/code/test_source.py 95.53% <0%> (-1.07%) ⬇️
testing/test_argcomplete.py 68.11% <0%> (-0.9%) ⬇️
testing/test_compat.py 91.13% <0%> (-0.63%) ⬇️
testing/test_skipping.py 97.65% <0%> (-0.57%) ⬇️
src/_pytest/warnings.py 88.88% <0%> (-0.53%) ⬇️
src/_pytest/debugging.py 77.98% <0%> (-0.41%) ⬇️
src/_pytest/fixtures.py 97.91% <0%> (-0.31%) ⬇️
src/_pytest/logging.py 94.96% <0%> (-0.31%) ⬇️
... and 74 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c7bbb2a...9cb71af. Read the comment docs.

@blueyed
Copy link
Contributor Author

blueyed commented Mar 1, 2019

Fails quite badly, e.g.

_______________________________ test_hello[2-5] ________________________________
arg1 = 2, arg2 = 5
    def test_hello(arg1, arg2):
>       assert 0, (arg1, arg2)
E       AttributeError: module '_pytest.assertion.rewrite' has no attribute '_format_explanation'
test_parametrize_functional2.py:5: AttributeError
=========================== 4 failed in 0.07 seconds ===========================

It is not clear to me how this works / is being called from - but at least it can be moved to the top it seems.

@blueyed blueyed changed the title Remove needless import in _pytest.assertion.rewrite Move import of _format_explanation in _pytest.assertion.rewrite Mar 1, 2019
@nicoddemus
Copy link
Member

Oh then it is probably being used by the rewritten AST.

@nicoddemus
Copy link
Member

And yes it can be moved to the top. 👍

@blueyed
Copy link
Contributor Author

blueyed commented Mar 1, 2019

Oh then it is probably being used by the rewritten AST.

Ah.. via https://github.com/blueyed/pytest/blob/9e9b140806b2a525ef0f830b627511b78830aea7/src/_pytest/assertion/rewrite.py#L857.

@blueyed blueyed merged commit 42561db into pytest-dev:features Mar 1, 2019
@blueyed blueyed deleted the remove-import branch March 1, 2019 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants