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.

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

Document how to test a deprecated class #8478

Open
twisted-trac opened this issue Jun 16, 2016 · 6 comments · May be fixed by #11971
Open

Document how to test a deprecated class #8478

twisted-trac opened this issue Jun 16, 2016 · 6 comments · May be fixed by #11971

Comments

@twisted-trac
Copy link

adiroiban's avatar @adiroiban reported
Trac ID trac#8478
Type enhancement
Created 2016-06-16 09:12:02Z
Branch https://github.com/twisted/twisted/tree/8478-deprecated-class-test

see the discussion from here http://twistedmatrix.com/pipermail/twisted-python/2016-June/030438.html

Searchable metadata
trac-id__8478 8478
type__enhancement enhancement
reporter__adiroiban adiroiban
priority__normal normal
milestone__None None
branch__8478_deprecated_class_test 8478-deprecated-class-test
branch_author__ 
status__new new
resolution__None None
component__core core
keywords__None None
time__1466068322246097 1466068322246097
changetime__1495180150090254 1495180150090254
version__None None
owner__markrwilliams markrwilliams

@twisted-trac
Copy link
Author

adiroiban's avatar @adiroiban removed owner

Please check the associated branch add see if it make sense.

I have updated the documentation to talk about how to test deprecated module attributes.

I have re-arrange the current docs so that the example about how to run trial so that it fails on warnings has a greater visibility.

Thanks!

@twisted-trac
Copy link
Author

mesozoic's avatar @mesozoic commented

This doesn't quite fix it, I think; test_RequestAvatarId_hashed would still cause an error because it's accessing a deprecated module attribute. It also bothers me to include hard-coded warning strings in our tests.

As you'd requested, I'm going to try to address this in a documentation patch as part of #8368. I'm also going to add a new test helper method, getDeprecatedModuleAttribute, which will retrieve the module attribute and make assertions about its deprecation status.

@twisted-trac
Copy link
Author

adiroiban's avatar @adiroiban commented

Hi mesozoic,
Feel free to push you changed to the associated branch.

My problem was that there were no examples on how to "properly" deprecate and test a class so I went for creating this branch to discuss and document what we agreed.

Cheers

@twisted-trac
Copy link
Author

rodrigc's avatar @rodrigc set owner to @adiroiban

This document is not fully consistent with the docstring in twisted/python/deprecate.py for
how to deprecate fields, methods, classes and modules.

I suggest that both the document and the docstring be made consistent.

@twisted-trac
Copy link
Author

markrwilliams's avatar @markrwilliams set owner to @markrwilliams

@twisted-trac
Copy link
Author

adiroiban's avatar @adiroiban commented

Once comment about testing deprecation.

When we outsource a module, we should keep all the existing tests as they are in order to make sure that the API is not changed.

We should not evade the deprecation policy by outsourcing the package... the outsource packages should have the same quality standards as the Twisted project itself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants