Opened 5 years ago

Closed 5 years ago

#6316 enhancement closed fixed (fixed)

Minor cleanup of pam tests in `twisted.test.test_newcred`.

Reported by: Tom Prince Owned by:
Priority: normal Milestone:
Component: core Keywords: easy
Cc: Branch: branches/pam-test-cleanup-6316
branch-diff, diff-cov, branch-cov, buildbot
Author: tomprince

Description

There is a pyflakes error, as well as an open-coded TestCase.patch. This cleans them up.

Change History (8)

comment:1 Changed 5 years ago by Tom Prince

Author: → tomprince
Branch: → branches/pam-test-cleanup-6316

(In [37165]) Branching to pam-test-cleanup-6316.

comment:2 Changed 5 years ago by Tom Prince

Keywords: easy review added

The official site for PyPAM seems to be down. I grabbed a copy for testing from the ​gentoo mirrors.

Note, these tests don't really do anything useful (See #3278)

comment:3 Changed 5 years ago by Jean-Paul Calderone

#3278 is a ticket about AMP.

comment:4 Changed 5 years ago by Tom Prince

I guess I meant #3728.

comment:5 Changed 5 years ago by Richard Wall

Owner: set to Richard Wall
Status: new → assigned

Reviewing...

comment:6 Changed 5 years ago by Richard Wall

Keywords: review removed
Owner: Richard Wall deleted
Status: assigned → new

Code Review:

  • ​Build Results look ok - some warnings because branch is out of date
  • Merges cleanly to trunk
  • The pyflakes unused import warning has gone. 
     [richard@zorin trunk]$ pyflakes twisted/test/test_newcred.py
 twisted/test/test_newcred.py:21: redefinition of unused 'crypt' from line 19
 twisted/test/test_newcred.py:24: 'callIntoPAM' imported but unused

 [richard@zorin pam-test-cleanup-6316]$ pyflakes twisted/test/test_newcred.py
 twisted/test/test_newcred.py:21: redefinition of unused 'crypt' from line 19
 twisted/test/test_newcred.py:26: redefinition of unused 'pamauth' from line 24
  • I can't actually test this.
    • pamauth expects a "PAM" package. But python-pam provides a "pam" package 
      >>> from twisted.cred import pamauth
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "twisted/cred/pamauth.py", line 9, in <module>
    import PAM
ImportError: No module named PAM
    • This launchpad bug ​https://bugs.launchpad.net/keystone/+bug/938801 suggests that certain distros provide a PAM package...but I can't install that either ...pycon wireless issues.

One comment:

  1. You're still saving the original method? Doesn't patch do all that for you? 
        self._oldCallIntoPAM = pamauth.callIntoPAM
    pamauth.callIntoPAM = self.callIntoPAM

Merge after removing those unnecessary lines.

-RichardW.

comment:7 Changed 5 years ago by Tom Prince

I believe ​this is the binding.

comment:8 Changed 5 years ago by Tom Prince

Resolution: → fixed
Status: new → closed

(In [37817]) Merge pam-test-cleanup-6316: Cleanup pam tests.

Author: tom.prince Reviewers: rwall Fixes: #6316

There is a pyflakes error, as well as an open-coded TestCase.patch. This cleans them up.

Note: See TracTickets for help on using tickets.