Skip to content

AttributeError chaining bug #1944 fix - #1945

Merged
The-Compiler merged 3 commits into
pytest-dev:masterfrom
axil:master
Sep 20, 2016
Merged

The-Compiler merged 3 commits into
pytest-dev:masterfrom
axil:master

Conversation

@axil

@axil axil commented Sep 18, 2016 •

Copy link
Copy Markdown

Thanks for submitting a PR, your contribution is really appreciated!

Here's a quick checklist that should be present in PRs:

  • Target: for bug or doc fixes, target master; for new features, target features;
  • Make sure to include one or more tests for your change;
  • Add yourself to AUTHORS;
  • Add a new entry to CHANGELOG.rst
    • Choose any open position to avoid merge conflicts with other PRs.
    • Add a link to the issue you are fixing (if any) using RST syntax.
    • The pytest team likes to have people to acknowledged in the CHANGELOG, so please add a thank note to yourself ("Thanks @user for the PR") and a link to your GitHub profile. It may sound weird thanking yourself, but otherwise a maintainer would have to do it manually before or after merging instead of just using GitHub's merge button. This makes it easier on the maintainers to merge PRs.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0008%) to 93.078% when pulling cfc955c on axil:master into 2bafa74 on pytest-dev:master.

@nicoddemus

Copy link
Copy Markdown
Member

Thanks, I've seen this happen on occasion but never took the time to properly look it up.

Would you mind add a CHANGELOG entry as well?

@axil

axil commented Sep 18, 2016

Copy link
Copy Markdown
Author

@nicoddemus done

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0008%) to 93.078% when pulling 37ccadf on axil:master into 2bafa74 on pytest-dev:master.

@The-Compiler

Copy link
Copy Markdown
Member

Thanks! Maybe it'd make sense to include a test which creates a file like the one in the issue, runs pytest over it, and checks that "AttributeError" isn't in the output? Should be really straightforward to add and prevent regressions for this in the future.

@Stranger6667 Stranger6667 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you guys for fixing this issue :)

@axil

axil commented Sep 19, 2016

Copy link
Copy Markdown
Author

Added a test, rebased against master.

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0008%) to 93.078% when pulling bb15acb on axil:master into 8639bf7 on pytest-dev:master.

@nicoddemus

Copy link
Copy Markdown
Member

Can you please take a look at the failing builds?

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.0008%) to 93.078% when pulling 8cba033 on axil:master into 8639bf7 on pytest-dev:master.

@axil

axil commented Sep 19, 2016

Copy link
Copy Markdown
Author

All passed now.

@The-Compiler

Copy link
Copy Markdown
Member

Thanks!

@The-Compiler
The-Compiler merged commit 7660a19 into pytest-dev:master Sep 20, 2016
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.

6 participants