From 7488cec43e34a51ba4a8439bb65a27590e07f3a2 Mon Sep 17 00:00:00 2001 From: Eisuke Kawashima Date: Mon, 25 Apr 2022 16:27:35 +0900 Subject: [PATCH 1/2] test(python): fix misuse of unittest.TestCase.assert_ --- test/testobconv_writers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/testobconv_writers.py b/test/testobconv_writers.py index 51e12ea54..e7e731e04 100644 --- a/test/testobconv_writers.py +++ b/test/testobconv_writers.py @@ -108,7 +108,7 @@ def get_mol(test_case, mol): mol = ob.OBMol() if not _smi_conv.ReadString(mol, smiles): - test_case.assert_("Cannot parse SMILES %r" % (smiles,)) + test_case.fail("Cannot parse SMILES %r" % (smiles,)) mol.SetTitle(title) return mol @@ -120,7 +120,7 @@ def get_mol(test_case, mol): def get_converter(test_case, output_format, options=None): conv = ob.OBConversion() if not conv.SetInAndOutFormats("smi", output_format): - test_case.assert_("Cannot set output format %r" % (output_format,)) + test_case.fail("Cannot set output format %r" % (output_format,)) if options: # Can pass in a dictionary ... From 0f4af753d466d9c548da09e50794d5c065a61aaf Mon Sep 17 00:00:00 2001 From: Eisuke Kawashima Date: Mon, 25 Apr 2022 15:42:18 +0900 Subject: [PATCH 2/2] test(python): improve usages of unitest.TestCase methods --- scripts/python/examples/dalke_test.py | 68 +++++++++++++-------------- scripts/python/examples/testpybel.py | 17 +++---- test/testbabel.py | 16 +++---- test/testbindings.py | 32 ++++++------- test/testcdjsonformat.py | 8 ++-- test/testpcjsonformat.py | 6 +-- test/testroundtrip.py | 6 +-- 7 files changed, 77 insertions(+), 76 deletions(-) diff --git a/scripts/python/examples/dalke_test.py b/scripts/python/examples/dalke_test.py index 0caaf9c73..0664eae74 100644 --- a/scripts/python/examples/dalke_test.py +++ b/scripts/python/examples/dalke_test.py @@ -22,13 +22,14 @@ def testfile(name): class MyTestCase(unittest.TestCase): def assertClose(self, val, expect): - if expect > 0: - self.assertTrue((expect * 0.9999) < val < (expect * 1.0001), val) - else: - self.assertTrue((expect * 1.0001) < val < (expect * 0.9999), val) + a, b = 0.9999, 1.0001 + if expect < 0: + a, b = b, a + self.assertLess(expect * a, val, val) + self.assertLess(val, expect * b, val) def assertZero(self, val): - self.assertTrue(abs(val) < 0.00001, val) + self.assertLess(abs(val), 0.00001, val) # Make a temporary directory for use during the "with" context block. @@ -137,8 +138,7 @@ class TestIO(MyTestCase): conv.CloseOutFile() lines = open(tempdir("blah.smi"), "U").readlines() - self.assertTrue(lines[0] == "CCO\t#1\n" or - lines[0] == "OCC\t#1\n", repr(lines[0])) + self.assertIn(lines[0], ["CCO\t#1\n", "OCC\t#1\n"], repr(lines[0])) self.assertTrue(lines[1] == "[NH4+]\tmol2\n", repr(lines[1])) def test_write_sdf(self): @@ -199,28 +199,28 @@ class TestPlugins(MyTestCase): def test_known_types(self): for name in TestPlugins.known_types: s = ob.OBPlugin.ListAsString(name) - self.assertFalse("not a recognized" in s, s) + self.assertNotIn("not a recognized", s, s) v = ob.vectorString() ob.OBPlugin.ListAsVector(name, None, v) - self.assertTrue(len(v) > 0, list(v)) + self.assertGreater(len(v), 0, list(v)) def test_as_string(self): s = ob.OBPlugin.ListAsString("fingerprints") - self.assertTrue("FP2" in s, s) - self.assertTrue("FP3" in s, s) - self.assertTrue("MACCS" in s, s) + self.assertIn("FP2", s, s) + self.assertIn("FP3", s, s) + self.assertIn("MACCS", s, s) def test_as_string_unknown_type(self): s = ob.OBPlugin.ListAsString("qwerty.shrdlu") - self.assertTrue("\nfingerprints\n" in s, s) - self.assertTrue("\nloaders\n" in s, s) + self.assertIn("\nfingerprints\n", s, s) + self.assertIn("\nloaders\n", s, s) def test_as_vector(self): v = ob.vectorString() ob.OBPlugin.ListAsVector("formats", None, v) formats = set(v) - self.assertTrue("smiles -- SMILES format" in formats, formats) + self.assertIn("smiles -- SMILES format", formats, formats) ## def test_list(self): ## # XXX GRR! To capture requires passing a 3rd argument which is a std:ostream @@ -243,7 +243,7 @@ class TestFingerprints(MyTestCase): ("FP4", "SMARTS patterns specified in the file SMARTS_InteLigand.txt" + P), ("MACCS", "SMARTS patterns specified in the file MACCS.txt" + P)): fingerprinter = ob.OBFingerprint.FindFingerprint(name) - self.assertFalse(fingerprinter is None) + self.assertIsNotNone(fingerprinter) self.assertEqual(fingerprinter.GetID(), name) self.assertEqual(fingerprinter.Description(), expected_description) # Which supported platforms have non-32-bit integers? @@ -489,8 +489,8 @@ class TestSmarts(MyTestCase): self.assertEqual(pat.Match(mol, v), 1) self.assertEqual(len(v), 2) results = list(v) - self.assertTrue((5, 6, 7) in results, results) - self.assertTrue((1, 6, 7) in results, results) + self.assertIn((5, 6, 7), results, results) + self.assertIn((1, 6, 7), results, results) def test_vector_match_with_one_unique_hit(self): mol = parse_smiles("c1ccccc1O") @@ -507,7 +507,7 @@ class TestSmarts(MyTestCase): self.assertEqual(pat.Match(mol, v, ob.OBSmartsPattern.Single), 1) self.assertEqual(len(v), 1) result = v[0] - self.assertTrue(result == (5, 6, 7) or result == (1, 6, 7), result) + self.assertIn(result, ((5, 6, 7), (1, 6, 7)), result) def test_vector_match_with_all_hits(self): mol = parse_smiles("c1ccccc1O") @@ -548,21 +548,21 @@ class TestDescriptors(MyTestCase): #mol.AddHydrogens() # doesn't change the results logp = calc_logp.Predict(mol) - self.assertTrue(abs(logp - 1.4008) <= 0.0001, logp) + self.assertLessEqual(abs(logp - 1.4008), 0.0001, logp) def test_tpsa(self): calc_tpsa = ob.OBDescriptor.FindType("TPSA") mol = parse_smiles("Oc1ccccc1OC") #mol.AddHydrogens() # doesn't change the results tpsa = calc_tpsa.Predict(mol) - self.assertTrue(abs(tpsa - 29.460) <= 0.001, tpsa) + self.assertLessEqual(abs(tpsa - 29.460), 0.001, tpsa) def test_mr(self): calc_mr = ob.OBDescriptor.FindType("MR") mol = parse_smiles("Oc1ccccc1OC") #mol.AddHydrogens() # doesn't change the results mr = calc_mr.Predict(mol) - self.assertTrue(abs(mr - 34.957) <= 0.001, mr) + self.assertLessEqual(abs(mr - 34.957), 0.001, mr) def test_gotta_try_them_all(self): v = ob.vectorString() @@ -571,7 +571,7 @@ class TestDescriptors(MyTestCase): for term in v: name = term.split()[0] prop_calculator = ob.OBDescriptor.FindType(name) - self.assertFalse(prop_calculator is None, "Could not find " + name) + self.assertIsNotNone(prop_calculator, "Could not find " + name) prop_calculator.Predict(mol) @@ -702,9 +702,9 @@ class TestAtomAndBond(MyTestCase): mol = parse_smiles("[12CH4-]") mol.SetTitle("Spam!") atom = mol.GetAtom(0) - self.assertTrue(atom is None, "GetAtom(0)") + self.assertIsNone(atom, "GetAtom(0)") atom = mol.GetAtom(1) - self.assertTrue(atom is not None, "GetAtom(1)") + self.assertIsNotNone(atom, "GetAtom(1)") self.assertEqual(atom.GetAtomicNum(), 6) self.assertEqual(atom.GetIsotope(), 12) @@ -788,7 +788,7 @@ class TestAtomAndBond(MyTestCase): self.assertClose(atom.GetPartialCharge(), -0.25658) - self.assertTrue(atom.GetParent().GetTitle() == mol.GetTitle(), + self.assertEqual(atom.GetParent().GetTitle(), mol.GetTitle(), "parent is mol") self.assertFalse(atom.IsAromatic()) @@ -816,7 +816,7 @@ class TestAtomAndBond(MyTestCase): C = mol.GetAtom(1) N = mol.GetAtom(2) # XXX Why do bonds starts from 0 and not 1 - self.assertTrue(mol.GetBond(1) is None) + self.assertIsNone(mol.GetBond(1)) bond = mol.GetBond(0) self.assertEqual(bond.GetLength(), 0.0) @@ -1041,7 +1041,7 @@ class TestAtomAndBond(MyTestCase): class SpectorphoreTest(MyTestCase): def assertWithin_0_001(self, val, expect): assert val > 0 - self.assertTrue(abs(val - expect) < 0.001, val) + self.assertLess(abs(val - expect), 0.001, val) def _make_mol(self): mol = ob.OBMol() def new_atom(eleno): @@ -1147,19 +1147,19 @@ class TestForceFields(MyTestCase): # Huh. The plugin system uses case-insensitive lookup names = [x.split()[0].lower() for x in v] - self.assertTrue("gaff" in names, names) - self.assertTrue("mmff94" in names, names) - self.assertTrue("uff" in names, names) + self.assertIn("gaff", names, names) + self.assertIn("mmff94", names, names) + self.assertIn("uff", names, names) pFF1 = ob.OBForceField.FindForceField("GAFF") pFF2 = ob.OBForceField.FindForceField("GafF") - self.assertFalse(pFF1 is None) - self.assertFalse(pFF2 is None) + self.assertIsNotNone(pFF1) + self.assertIsNotNone(pFF2) self.assertEqual(pFF1.GetID(), pFF2.GetID()) def _test_energies(self, plugin_name, expected_results, filename = None): pFF = ob.OBForceField.FindForceField(plugin_name) - self.assertFalse(pFF is None, "Cannot load " + plugin_name) + self.assertIsNotNone(pFF, "Cannot load " + plugin_name) if filename is None: filename = testfile("forcefield.sdf") diff --git a/scripts/python/examples/testpybel.py b/scripts/python/examples/testpybel.py index e674c8ada..13e5d0435 100644 --- a/scripts/python/examples/testpybel.py +++ b/scripts/python/examples/testpybel.py @@ -256,7 +256,7 @@ M END # (even those that are supposed to be immune like TPSA) self.mols[1].addh() desc = self.mols[1].calcdesc() - self.assertTrue(len(desc) > 3) + self.assertGreater(len(desc), 3) self.assertAlmostEqual(desc[self.tpsaname], 26.02, 2) self.assertRaises(ValueError, self.RFdesctest) @@ -273,24 +273,25 @@ M END newvalues = {'hey':'there', 'yo':1} data.update(newvalues) self.assertEqual(data['yo'], '1') - self.assertTrue('there' in data.values()) + self.assertIn('there', data.values()) def testMDglobalaccess(self): """Check out the keys""" data = self.head[0].data - self.assertFalse('Noel' in data) + self.assertNotIn('Noel', data) self.assertEqual(len(data), len(self.datakeys)) for key in data: - self.assertEqual(key in self.datakeys, True) + self.assertIn(key, self.datakeys) r = repr(data) - self.assertTrue(r[0]=="{" and r[-2:]=="'}", r) + self.assertEqual(r[0], "{", r) + self.assertEqual(r[-2:], "'}", r) def testMDdelete(self): """Delete some keys""" data = self.head[0].data - self.assertTrue('NSC' in data) + self.assertIn('NSC', data) del data['NSC'] - self.assertFalse('NSC' in data) + self.assertNotIn('NSC', data) data.clear() self.assertEqual(len(data), 0) @@ -358,7 +359,7 @@ class TestPybel(TestToolkit): def testMDcomment(self): """Mess about with the comment field""" data = self.head[0].data - self.assertEqual('Comment' in data, True) + self.assertIn('Comment', data) self.assertEqual(data['Comment'], 'CORINA 2.61 0041 25.10.2001') data['Comment'] = 'New comment' self.assertEqual(data['Comment'], 'New comment') diff --git a/test/testbabel.py b/test/testbabel.py index 479e20972..863abf379 100644 --- a/test/testbabel.py +++ b/test/testbabel.py @@ -115,15 +115,15 @@ class TestOBabel(BaseTest): """Ensure that this does not segfault (PR#1818)""" self.canFindExecutable("obabel") output, error = run_exec("obabel -i") - self.assertTrue(len(output) > 1, "Did not generate output") - self.assertTrue(len(error) > 1, "Did not generate error message") + self.assertGreater(len(output), 1, "Did not generate output") + self.assertGreater(len(error), 1, "Did not generate error message") def testShortOutfileName(self): # Test that -O can handle short file names # if not the command will show warning on 2D coords self.canFindExecutable("obabel") _, errormsg = run_exec("CCC", "obabel -ismi -omol -Otf --gen2D") - self.assertFalse("No 2D or 3D coordinates" in errormsg) + self.assertNotIn("No 2D or 3D coordinates", errormsg) def testSMItoInChI(self): self.canFindExecutable("obabel") @@ -142,7 +142,7 @@ class TestOBabel(BaseTest): errors = ["reactant", "agent", "product"] for rsmi, error in zip(data, errors): output, errormsg = run_exec('obabel -:%s -irsmi -orsmi' % rsmi) - self.assertTrue(error in errormsg) + self.assertIn(error, errormsg) def sort(self, rsmi): # TODO: Change OBMol.Separate to preserve the order. This @@ -164,10 +164,10 @@ class TestOBabel(BaseTest): "c%(000001)ccccc%(000001)", "c%(51)ccccc%(15)"] for smi in data: output, error = run_exec("obabel -:%s -osmi" % smi) - self.assertTrue("0 molecules converted" in error) + self.assertIn("0 molecules converted", error) # Now test writing of %(NNN) notation output, error = run_exec("obabel %s -osmi" % self.getTestFile("102Uridine.smi")) - self.assertTrue("%(100)" in output) + self.assertIn("%(100)", output) def testPDBQT(self): self.canFindExecutable("obabel") @@ -250,7 +250,7 @@ TORSDOF 5 else: os.environ.pop("BABEL_LIBDIR") - self.assertTrue('BABEL_LIBDIR' in msg) + self.assertIn('BABEL_LIBDIR', msg) def testCOFtoCAN(self): self.canFindExecutable("obabel") @@ -430,7 +430,7 @@ TORSDOF 5 other mol2 test here''' mol2file = self.getTestFile('5sun_protein.mol2') outputerr = run_exec( "obabel -imol2 %s -osdf" % mol2file) - self.assertTrue(len(outputerr[0]) > 0, "Did not generate output") + self.assertGreater(len(outputerr[0]), 0, "Did not generate output") if __name__ == "__main__": diff --git a/test/testbindings.py b/test/testbindings.py index d29ffa9c9..5f30f458c 100644 --- a/test/testbindings.py +++ b/test/testbindings.py @@ -39,7 +39,7 @@ except ImportError: class PythonBindings(unittest.TestCase): def setUp(self): - self.assertTrue(ob is not None, "Failed to import the openbabel module") + self.assertIsNotNone(ob, "Failed to import the openbabel module") class TestPythonBindings(PythonBindings): def testSimple(self): @@ -51,7 +51,7 @@ class TestPythonBindings(PythonBindings): class PybelWrapper(PythonBindings): def testDummy(self): - self.assertTrue(pybel is not None, "Failed to import the Pybel module") + self.assertIsNotNone(pybel, "Failed to import the Pybel module") class TestSuite(PythonBindings): @@ -208,7 +208,7 @@ $end""" self.assertEqual(res, atomorder) mol = pybel.readstring("smi", "CC") mol.write("can") - self.assertFalse("SMILES Atom Order" in mol.data) + self.assertNotIn("SMILES Atom Order", mol.data) def testECFP(self): data = [ @@ -224,7 +224,7 @@ $end""" ecfp2 = mol.calcfp("ecfp2").bits self.assertEqual(len(ecfp2), numB) for bit in ecfp0: - self.assertTrue(bit in ecfp2) + self.assertIn(bit, ecfp2) def testOldRingInformationIsWipedOnReperception(self): """Previously, the code that identified ring atoms and bonds @@ -380,10 +380,10 @@ H 0.74700 0.50628 -0.64089 changed = neutralize.Do(mol, option) result = pybel.Molecule(mol).write("smi").rstrip() self.assertEqual(ans, result) - if not option: - self.assertEqual(True, changed) + if not option or after: + self.assertTrue(changed) else: - self.assertEqual(True if after else False, changed) + self.assertFalse(changed) def testImplicitCisDblBond(self): """Ensure that dbl bonds in rings of size 8 or less are always @@ -393,12 +393,12 @@ H 0.74700 0.50628 -0.64089 ringsize = i + 4 ringsmi = smi + "1" roundtrip = pybel.readstring("smi", ringsmi).write("smi") - self.assertTrue("/" not in roundtrip) + self.assertNotIn("/", roundtrip) smi += "C" ringsize = 9 ringsmi = smi + "1" roundtrip = pybel.readstring("smi", ringsmi).write("smi") - self.assertTrue("/" in roundtrip) + self.assertIn("/", roundtrip) def testKekulizationOfHypervalents(self): # We should support hypervalent aromatic S and N (the latter @@ -638,7 +638,7 @@ H -0.26065 0.64232 -2.62218 # Check whether the element is available as a constant self.assertEqual(N, getattr(ob, ob.GetName(N))) - self.assertTrue(N > 100) + self.assertGreater(N, 100) def testElementsSpecifiedByAtomicNumberInSmiles(self): smis = [ @@ -678,7 +678,7 @@ H -0.26065 0.64232 -2.62218 for a, b in itertools.combinations(mol.atoms, 2) ] mindist = min(dists) - self.assertTrue(mindist > 0.00001) + self.assertGreater(mindist, 0.00001) def testRegressionBenzene2D(self): """Check that benzene is given a correct layout, see #1900""" @@ -884,16 +884,16 @@ class AcceptStereoAsGiven(PythonBindings): # Should preserve stereo tet = "[C@@H](Br)(Br)Br" out = pybel.readstring("smi", tet).write("smi") - self.assertTrue("@" in out) + self.assertIn("@", out) cistrans = r"C/C=C(\C)/C" out = pybel.readstring("smi", cistrans).write("smi") - self.assertTrue("/" in out) + self.assertIn("/", out) # Should wipe stereo out = pybel.readstring("smi", tet, opt={"S": True}).write("smi") - self.assertFalse("@" in out) + self.assertNotIn("@", out) cistrans = r"C/C=C(\C)/C" out = pybel.readstring("smi", cistrans, opt={"S": True}).write("smi") - self.assertFalse("/" in out) + self.assertNotIn("/", out) class OBMolCopySubstructure(PythonBindings): """Tests for copying a component of an OBMol""" @@ -1126,7 +1126,7 @@ class AtomClass(PythonBindings): smi = "[*:6]C" mol = pybel.readstring("smi", smi) molfile = mol.write("mol") - self.assertTrue("M RGP 1 1 6" in molfile) + self.assertIn("M RGP 1 1 6", molfile) molb = pybel.readstring("mol", molfile) out = mol.write("smi", opt={"a":True, "n":True, "nonewline":True}) self.assertEqual(smi, out) diff --git a/test/testcdjsonformat.py b/test/testcdjsonformat.py index 4948ca653..f5ff7a397 100644 --- a/test/testcdjsonformat.py +++ b/test/testcdjsonformat.py @@ -74,8 +74,8 @@ class TestCdJsonFormat(PybelWrapper): output = json.loads(mols[0].write('cdjson')) self.assertEqual(len(output['m'][0]['a']), 4) for a in output['m'][0]['a']: - self.assertTrue('x' in a) - self.assertTrue('y' in a) + self.assertIn('x', a) + self.assertIn('y', a) def test_write_bonds(self): """Test writing bonds.""" @@ -87,9 +87,9 @@ class TestCdJsonFormat(PybelWrapper): """Test writing minified output.""" mols = list(pybel.readfile("cdjson", os.path.join(filedir, 'butane.json'))) output = mols[0].write('cdjson', opt={'m': None}) - self.assertTrue('\n' not in output) + self.assertNotIn('\n', output) output = mols[0].write('cdjson') - self.assertTrue('\n' in output) + self.assertIn('\n', output) if __name__ == "__main__": diff --git a/test/testpcjsonformat.py b/test/testpcjsonformat.py index e74edf332..580e0a3c8 100644 --- a/test/testpcjsonformat.py +++ b/test/testpcjsonformat.py @@ -83,7 +83,7 @@ class TestPcJsonFormat(PybelWrapper): mols = list(pybel.readfile("pcjson", os.path.join(filedir, 'CID_2244_2D.json'))) output = json.loads(mols[0].write('pcjson')) self.assertEqual(len(output['PC_Compounds']), 1) - self.assertTrue('id' in output['PC_Compounds'][0]) + self.assertIn('id', output['PC_Compounds'][0]) self.assertEqual(output['PC_Compounds'][0]['id']['id']['cid'], '2244') def test_write_atoms(self): @@ -99,9 +99,9 @@ class TestPcJsonFormat(PybelWrapper): """Test writing minified output.""" mols = list(pybel.readfile("pcjson", os.path.join(filedir, 'CID_6857552_2D.json'))) output = mols[0].write('pcjson', opt={'m': None}) - self.assertTrue('\n' not in output) + self.assertNotIn('\n', output) output = mols[0].write('pcjson') - self.assertTrue('\n' in output) + self.assertIn('\n', output) def test_write_complex_bonds(self): """Test writing complex bonds.""" diff --git a/test/testroundtrip.py b/test/testroundtrip.py index afabc316b..d67b8de7a 100644 --- a/test/testroundtrip.py +++ b/test/testroundtrip.py @@ -174,7 +174,7 @@ def roundtripFile(fname): class TestSuite(unittest.TestCase): def setUp(self): - self.assertTrue(ob is not None, "Failed to import the openbabel module") + self.assertIsNotNone(ob, "Failed to import the openbabel module") def canFindFile(self, filename): self.assertTrue(os.path.exists(filename), @@ -210,11 +210,11 @@ class TestSuite(unittest.TestCase): if ret != None: print(i,fname) print(ret) - self.assertTrue(ret == None, ret) + self.assertIsNone(ret, ret) except TimeoutError: print(i,fname) print("Timeout or segfault") - self.assertTrue(False,"Timeout or segfault with %s"%fname) + self.fail("Timeout or segfault with %s"%fname)