Skip to content

Refactor test_recwarn into functional tests instead of testing private implementation #2298

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

Closed

Conversation

nicoddemus
Copy link
Member

Replace tests which test implementation details by functional tests, so we can refactor recwarn module
to simplify it by using the warnings.catch_warnings builtin, available since Python 2.6.

I will open a new PR based upon this one with the refactor itself; I'm opening in two PRs to prove that the behavior has not changed, since I had to convert the tests to functional tests first.

@@ -91,18 +94,6 @@ def test_deprecated_call_ret(self):
ret = pytest.deprecated_call(self.dep, 0)
assert ret == 42

def test_deprecated_call_preserves(self):
Copy link
Member Author

Choose a reason for hiding this comment

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

The test is about testing the internal implementation, so I removed it.

@@ -225,9 +216,6 @@ def test_record(self):
assert len(record) == 1
assert str(record[0].message) == "user"

print(repr(record[0]))
Copy link
Member Author

Choose a reason for hiding this comment

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

This was tested the recent change of the WarningMessage object to a namedtuple, but this is not part of the API and has never been published, so it is safe to go.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 92.763% when pulling 3a5231e on nicoddemus:recwar-functional-tests into 0c94f51 on pytest-dev:features.

@nicoddemus
Copy link
Member Author

Just realized that I can make WarningsRecorder subclass warnings.catch_warnings (it's an object) so it is possible to keep the exact same interface as before and still do the refactor I want, so changing the tests to functional tests is no longer a requirement for me.

Thoughts?

…e implementation

Replace tests which test implementation details by functional tests, so we can refactor recwarn module
to simplify it by using the warnings.catch_warnings builtin, available since Python 2.6.
@nicoddemus nicoddemus force-pushed the recwar-functional-tests branch from 3a5231e to 679e92b Compare March 12, 2017 22:17
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 92.642% when pulling 679e92b on nicoddemus:recwar-functional-tests into d1ea7c8 on pytest-dev:features.

@RonnyPfannschmidt
Copy link
Member

i strongly dislike how we turn all kinds of local component tests into system tests, its pretty much worst practice

@nicoddemus
Copy link
Member Author

I agree, I was just changing them into functional because my initial take had removed the internal classes; but then afterwards I was able to keep the exact same interface as before, so we can keep the tests as they are right now.

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.

3 participants