Revamp PDBFixer testing (#225)

* Revamp PDBFixer testing

* Use GitHub Actions to run a suite of tests
* Switch from nose to pytest (nose has not been supported in years)
* Download a PDB file and track it instead of fetching it during CI
  (RCSB has blocked testing frameworks in the past).

* Add missing environment.yaml file

* Try the workflow on my branch

* Try to fix the pipeline

* Stop building on jms-updates

.github/workflows/CI.yml

+name: PDBFixer Continuous Integration Workflow
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  build_test:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: [3.7, 3.8, 3.9]
+
+    steps:
+      - uses: actions/checkout@v2
+        name: "Checkout the source code"
+
+      - uses: conda-incubator/setup-miniconda@v2
+        name: "Prepare base dependencies"
+        with:
+          python-version: ${{ matrix.python-version }}
+          activate-environment: pdbfixer-dev
+          environment-file: devtools/environment-dev.yaml
+
+      - name: "Install PDBFixer"
+        shell: bash -l {0}
+        run: |
+          which python
+          python -V
+          pip install -e .
+          pytest -vs pdbfixer

devtools/environment-dev.yaml

+name: pdbfixer-dev
+
+channels:
+  - conda-forge
+
+dependencies:
+  - pytest
+  - openmm
+  - numpy
+  - pip
\ No newline at end of file

pdbfixer/tests/test_build_and_simulate.py

 
 from __future__ import print_function
 import pdbfixer
-import simtk.openmm 
+import openmm
 
 import os
 import os.path
@@ -29,9 +29,9 @@ class Watchdog(BaseException):
         raise self
 
 def simulate(pdbcode, pdb_filename):
-    from simtk.openmm import app
-    import simtk.openmm as mm
-    from simtk import unit
+    from openmm import app
+    import openmm.openmm as mm
+    from openmm import unit
     from sys import stdout
 
     # Load the PDB file.

pdbfixer/tests/test_cli.py

@@ -13,10 +13,11 @@ Test command-line interface.
 # GLOBAL IMPORTS
 #=============================================================================================
 
+import os
 import subprocess
 from subprocess import CalledProcessError
 
-from nose.plugins.skip import Skip, SkipTest
+import pytest
 
 #=============================================================================================
 # UNIT TESTS
@@ -50,5 +51,6 @@ def test_help():
 def test_pdbid():
     run_cli('--pdbid 1LE1')
 
+@pytest.mark.skipif(os.getenv("GITHUB_ACTION") is not None, reason="Cannot download during CI")
 def test_url():
     run_cli('--url "http://www.rcsb.org/pdb/download/downloadFile.do?fileFormat=pdb&compression=NO&structureId=1LE1"')

pdbfixer/tests/test_mutate.py

-from nose.tools import ok_, eq_, raises
-import simtk.openmm.app as app
+import openmm.app as app
 import pdbfixer
 import tempfile
+from pytest import raises
 
 def test_mutate_1():
     fixer = pdbfixer.PDBFixer(pdbid='1VII')
@@ -48,21 +48,21 @@ def test_mutate_2():
     desired_atom_names = set(["C", "N", "CA", "CB", "CG", "CD1", "CD2", "O", "H", "HA", "HB2", "HB3", "HD11", "HD12", "HD13", "HD21", "HD22", "HD23", "HG"])
     assert atom_names == desired_atom_names, "Atom Names did not match for LEU 57"
 
-@raises(ValueError)
 def test_mutate_3_fails():
-    fixer = pdbfixer.PDBFixer(pdbid='1VII')
-    fixer.applyMutations(["ALA-57-GLY", "SER-57-ALA"], "A")
+    with raises(ValueError):
+        fixer = pdbfixer.PDBFixer(pdbid='1VII')
+        fixer.applyMutations(["ALA-57-GLY", "SER-57-ALA"], "A")
 
-@raises(KeyError)
 def test_mutate_4_fails():
-    fixer = pdbfixer.PDBFixer(pdbid='1VII')
-    fixer.applyMutations(["ALA-57-WTF", "SER-56-ALA"], "A")
+    with raises(KeyError):
+        fixer = pdbfixer.PDBFixer(pdbid='1VII')
+        fixer.applyMutations(["ALA-57-WTF", "SER-56-ALA"], "A")
 
 
-@raises(KeyError)
 def test_mutate_5_fails():
-    fixer = pdbfixer.PDBFixer(pdbid='1VII')
-    fixer.applyMutations(["ALA-1000-GLY", "SER-56-ALA"], "A")
+    with raises(KeyError):
+        fixer = pdbfixer.PDBFixer(pdbid='1VII')
+        fixer.applyMutations(["ALA-1000-GLY", "SER-56-ALA"], "A")
 
 def test_mutate_multiple_copies_of_chain_A():
     fixer = pdbfixer.PDBFixer(pdbid='1OL5')

pdbfixer/tests/test_removechains.py

-from nose.tools import ok_, eq_, raises, assert_list_equal
-import simtk.openmm.app as app
+import openmm.app as app
 import pdbfixer
 import tempfile
 import time
+from pathlib import Path
 
-try:
-    from urllib.request import urlopen
-    from io import StringIO
-except:
-    from urllib2 import urlopen
-    from cStringIO import StringIO
+from urllib.request import urlopen
+from io import StringIO
 
-# Download the file once to avoid repeated requests to the PDB.
+import pytest
 
-file_content = urlopen('http://www.rcsb.org/pdb/files/4JSV.pdb').read().decode('utf-8')
+@pytest.fixture(scope="module")
+def file_content():
+    return (Path(__file__).parent / "data" / "4JSV.pdb").read_text()
 
-def remove_chain_ids_and_verify(chain_ids_to_remove, expected_chain_ids_remaining):
+def remove_chains_and_verify(file_content, expected_chain_ids_remaining, **kws):
     # Create a PDBFixer instance for the given pdbid
     fixer = pdbfixer.PDBFixer(pdbfile=StringIO(file_content))
     # Remove specified chains.
-    fixer.removeChains(chainIds=chain_ids_to_remove)
+    fixer.removeChains(**kws)
     # Check to make sure asserted chains remain.
     chain_ids_remaining = [c.id for c in fixer.topology.chains()]
-    assert_list_equal(chain_ids_remaining, expected_chain_ids_remaining)
+    assert expected_chain_ids_remaining == chain_ids_remaining
 
-def test_removechain_ids():
-    remove_chain_ids_and_verify([], ['B', 'D', 'A', 'C', 'B', 'A'])
-    remove_chain_ids_and_verify(['B', 'D'], ['A', 'C', 'A'])
-    remove_chain_ids_and_verify(['A', 'C'], ['B', 'D', 'B'])
-    remove_chain_ids_and_verify(['B', 'A'], ['D', 'C'])
-    remove_chain_ids_and_verify(['B', 'D', 'A', 'C'], [])
+def test_removechain_ids(file_content):
+    remove_chains_and_verify(file_content, ['B', 'D', 'A', 'C', 'B', 'A'], chainIds=[])
+    remove_chains_and_verify(file_content, ['A', 'C', 'A'], chainIds=['B', 'D'])
+    remove_chains_and_verify(file_content, ['B', 'D', 'B'], chainIds=['A', 'C'])
+    remove_chains_and_verify(file_content, ['D', 'C'], chainIds=['B', 'A'])
+    remove_chains_and_verify(file_content, [], chainIds=['B', 'D', 'A', 'C'])
 
-def remove_chain_indices_and_verify(chain_indices_to_remove, expected_chain_ids_remaining):
-    # Create a PDBFixer instance for the given pdbid
-    fixer = pdbfixer.PDBFixer(pdbfile=StringIO(file_content))
-    # Remove specified chains.
-    fixer.removeChains(chainIndices=chain_indices_to_remove)
-    # Check to make sure asserted chains remain.
-    chain_ids_remaining = [c.id for c in fixer.topology.chains()]
-    assert_list_equal(chain_ids_remaining, expected_chain_ids_remaining)
-
-def test_removechain_indices():
-    remove_chain_indices_and_verify([], ['B', 'D', 'A', 'C', 'B', 'A'])
-    remove_chain_indices_and_verify([0, 1], ['A', 'C', 'B', 'A'])
-    remove_chain_indices_and_verify([2, 3], ['B', 'D', 'B', 'A'])
-    remove_chain_indices_and_verify([0, 2], ['D', 'C', 'B', 'A'])
-    remove_chain_indices_and_verify([0, 1, 2, 3, 4, 5], [])
+def test_removechain_indices(file_content):
+    remove_chains_and_verify(file_content, ['B', 'D', 'A', 'C', 'B', 'A'], chainIndices=[])
+    remove_chains_and_verify(file_content, ['A', 'C', 'B', 'A'], chainIndices=[0, 1])
+    remove_chains_and_verify(file_content, ['B', 'D', 'B', 'A'], chainIndices=[2, 3])
+    remove_chains_and_verify(file_content, ['D', 'C', 'B', 'A'], chainIndices=[0, 2])
+    remove_chains_and_verify(file_content, [], chainIndices=[0, 1, 2, 3, 4, 5])

setup.py

@@ -68,7 +68,7 @@ setup(
     long_description="\n".join(DOCLINES[2:]),
     version=__version__,
     license='MIT',
-    url='https://github.com/SimTk/pdbfixer',
+    url='https://github.com/openmm/pdbfixer',
     platforms=['Linux', 'Mac OS-X', 'Unix', 'Windows'],
     classifiers=CLASSIFIERS.splitlines(),
     packages=find_packages(),