Opened 5 years ago

Closed 4 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: newassigned

Reviewing...

comment:6 Changed 5 years ago by Richard Wall

Keywords: review removed
Owner: Richard Wall deleted
Status: assignednew

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 4 years ago by Tom Prince

I believe this is the binding.

comment:8 Changed 4 years ago by Tom Prince

Resolution: fixed
Status: newclosed

(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.