Fix pytest Discovery Bug (FooTest vs. TestFoo), Add Missing Tests, Fix Generator Tests
#2525
Conversation
We have we have classes called, for example, OutputTest, OutputUnitTest, ArkaneTestThermo, CommonTest, InputTest, Psi4LogTest, QChemLogTest, TeraChemLogTest, ArkaneGaussianLogTest, MolproLogTest, OrcaTest, ArkaneTest, ArkaneKineticsTest, FluxDiagramTest, SimulateTest, DiffModelsTest, GenerateReactionsTest, CanteraTest, RMGToCanteraTest, MergeModelsTest, regressionTest, IsotopesTest, ReactionSystemTest, to name just a few... They were being ignored by pytest. Maybe we should rename them, but for now lets reconfigure pytest.
|
Running locally, I get 68 failed, 1788 passed, 26 skipped, 34 deselected, 38 warnings in 617.62s (0:10:17). (But my cythonized binaries may be on a topic branch, so we'll see what the official CI gets.) |
|
😿 I chalked the difference in total number of tests up to differences in counting the database tests (generator vs expect) but it appears that was wrong. I will take a look now. |
tests are apparently modifying the molecule, need to 'reset' it after each test
|
The 'good' thing is that most of the failures were due to incorrect filepaths for files used in testing. Fixing and pushing now. |
broke the two arkane input tests into separate files, one which covers using arkane in a functional programming style and one which uses it through input files. The latter I was able to get working by adding global declarations, the former is broken in a similar way to the explorerTest, which is also skipped
|
@rwest should be mostly fixed now. Locally, this was passing all unit tests. One of the Arkane functional tests is not working anymore, and I don't know why. When running |
|
Sigh... that worked locally. Going to skip that test too. |
|
To clarify the reason for skipping, here is the error: _________________________ TestArkaneInput.test_species _________________________
self = <arkaneInputTest.TestArkaneInput object at 0x7f4fb3301210>
def test_species(self):
"""Test loading of species input file."""
global job_list
spec = input.species("C2H4", os.path.join(self.directory, "species", "C2H4", "ethene.py"))
assert isinstance(spec, Species)
assert len(spec.molecule) == 0
# statmech job test
> job = job_list[-1]
E IndexError: list index out of range
test/arkane/arkaneInputTest.py:64: IndexErrorAfter calling As I state in the |
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:34 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Failed Edge Comparison ❌Original model has 106 species. Non-identical thermo! ❌
thermo: Thermo group additivity estimation: group(Cs-(Cds-Cds)(Cds-Cds)(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)(Cds-Cds)) + group(Cds-CdsCsH) + group(Cds-CdsCsH) + group(Cds-Cds(Cds-Cds)H) + group(Cds-Cds(Cds-Cds)H) + group(Cds-CdsCsH) + group(Cdd-CdsCds) + Estimated bicyclic component: polycyclic(s4_6_6_ane) - ring(Cyclohexane) - ring(Cyclohexane) + ring(124cyclohexatriene) + ring(1,4-Cyclohexadiene) Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: Aromatics Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:03:11 liquid_oxidation Passed Core Comparison ✅Original model has 37 species. liquid_oxidation Failed Edge Comparison ❌Original model has 202 species. DetailsObservables Test Case: liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:01:55 nitrogen Failed Core Comparison ❌Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 132 species. DetailsObservables Test Case: NC Comparison✅ All Observables varied by less than 0.200 on average between old model and new model in all conditions! nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:03:14 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. DetailsObservables Test Case: Oxidation Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! oxidation Passed Observable Testing ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:01:18 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species. DetailsObservables Test Case: SO2 Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! sulfur Passed Observable Testing ✅Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:49 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Passed Edge Comparison ✅Original model has 18 species. Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:03:45 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species. DetailsObservables Test Case: RMS_constantVIdealGasReactor_superminimal Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:09:12 RMS_CSTR_liquid_oxidation Failed Core Comparison ❌Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 206 species. Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: RMS_CSTR_liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_CSTR_liquid_oxidation Passed Observable Testing ✅beep boop this comment was written by a bot 🤖 |
Codecov Report
@@ Coverage Diff @@
## main #2525 +/- ##
==========================================
+ Coverage 49.88% 55.32% +5.44%
==========================================
Files 125 125
Lines 37139 37139
==========================================
+ Hits 18526 20547 +2021
+ Misses 18613 16592 -2021 see 37 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Woo for +5% coverage boost. I think this is ready @rwest |
|
The file test/rmgpy/molecule/isomorphismTest.py is rather confusing. Can you reassure me that it's doing everything intended and not missing any generated tests? This run on the old nosetests contains The current pytest has the test method names in the latter seem to match up to the test descriptions in the former, apart from the first one which is Could there be other parameterized and generated tests that we're missing? |
|
Here's a thought. Could you check the coverage report of the |
some had Test in the wrong place in the class name, others were called Check instead, some had test_ in the wrong place in the method name. pytest was ignoring all of them, fixed now
|
I have fixed the isomorphism tests (405a76f), found a few more straggler tests (5c46ff6) by including |
JacksonBurns
changed the title
pytest Discovery Bug (FooTest vs. TestFoo), Add Missing Tests, Fix Generator Tests
Regression Testing Results
Detailed regression test results.Regression test aromatics:Reference: Execution time (DD:HH:MM:SS): 00:00:01:38 aromatics Passed Core Comparison ✅Original model has 15 species. aromatics Passed Edge Comparison ✅Original model has 106 species. DetailsObservables Test Case: Aromatics Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! aromatics Passed Observable Testing ✅Regression test liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:03:23 liquid_oxidation Failed Core Comparison ❌Original model has 37 species. Non-identical kinetics! ❌
kinetics: liquid_oxidation Failed Edge Comparison ❌Original model has 202 species. Non-identical kinetics! ❌
kinetics: Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! liquid_oxidation Passed Observable Testing ✅Regression test nitrogen:Reference: Execution time (DD:HH:MM:SS): 00:00:02:17 nitrogen Failed Core Comparison ❌Original model has 41 species. nitrogen Failed Edge Comparison ❌Original model has 132 species. DetailsObservables Test Case: NC Comparison✅ All Observables varied by less than 0.200 on average between old model and new model in all conditions! nitrogen Passed Observable Testing ✅Regression test oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:03:39 oxidation Passed Core Comparison ✅Original model has 59 species. oxidation Passed Edge Comparison ✅Original model has 230 species. DetailsObservables Test Case: Oxidation Comparison✅ All Observables varied by less than 0.500 on average between old model and new model in all conditions! oxidation Passed Observable Testing ✅Regression test sulfur:Reference: Execution time (DD:HH:MM:SS): 00:00:01:24 sulfur Passed Core Comparison ✅Original model has 27 species. sulfur Failed Edge Comparison ❌Original model has 89 species. DetailsObservables Test Case: SO2 Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! sulfur Passed Observable Testing ✅Regression test superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:00:57 superminimal Passed Core Comparison ✅Original model has 13 species. superminimal Passed Edge Comparison ✅Original model has 18 species. Regression test RMS_constantVIdealGasReactor_superminimal:Reference: Execution time (DD:HH:MM:SS): 00:00:03:59 RMS_constantVIdealGasReactor_superminimal Passed Core Comparison ✅Original model has 13 species. RMS_constantVIdealGasReactor_superminimal Passed Edge Comparison ✅Original model has 13 species. DetailsObservables Test Case: RMS_constantVIdealGasReactor_superminimal Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_constantVIdealGasReactor_superminimal Passed Observable Testing ✅Regression test RMS_CSTR_liquid_oxidation:Reference: Execution time (DD:HH:MM:SS): 00:00:09:03 RMS_CSTR_liquid_oxidation Passed Core Comparison ✅Original model has 37 species. RMS_CSTR_liquid_oxidation Failed Edge Comparison ❌Original model has 206 species. Non-identical kinetics! ❌
kinetics: DetailsObservables Test Case: RMS_CSTR_liquid_oxidation Comparison✅ All Observables varied by less than 0.100 on average between old model and new model in all conditions! RMS_CSTR_liquid_oxidation Passed Observable Testing ✅beep boop this comment was written by a bot 🤖 |
|
@rwest I think this is actually ready now |
rwest
left a comment
rwest
left a comment
There was a problem hiding this comment.
Thanks. Looks good to me. How confident are you this is all of them? 😆
(I authored the PR so it won't let me approve it)
Reasonably confident - anything else still hiding presumably can't be that destructive. |
2 participants
Motivation or Problem
Well this is a bit embarrassing. None of us noticed that the unit tests in the classes OutputTest, OutputUnitTest, ArkaneTestThermo, CommonTest, InputTest, Psi4LogTest, QChemLogTest, TeraChemLogTest, ArkaneGaussianLogTest, MolproLogTest, OrcaTest, ArkaneTest, ArkaneKineticsTest, FluxDiagramTest, SimulateTest, DiffModelsTest, GenerateReactionsTest, CanteraTest, RMGToCanteraTest, MergeModelsTest, regressionTest, IsotopesTest, ReactionSystemTest, ResonanceTest, ClarTest, InChITest, AugmentedInChITest, IgnorePrefixTest, ComposeTest, Parse_H_LayerTest, Parse_E_LayerTest, ParseNLayerTest, DecomposeTest, CreateULayerTest, ExpectedLonePairsTest, CreateAugmentedLayersTest, ResetLonePairsTest, FiltrationTest, FindButadieneTest, FindAllylEndWithChargeTest, FindButadieneEndWithChargeTest, ShortestPathTest, DistanceComputingTest, FindAllylDelocalizationPathsTest, FindLonePairMultipleBondPathsTest, RDKitTest, ConverterTest, TranslatorTest, InChIGenerationTest, SMILESGenerationTest, ParsingTest, InChIParsingTest, KekulizeTest, ElementCountTest, PartitionTest, AgglomerateTest, ComboGeneratorTest, SwapTest, were not being discovered or run by pytest, because they don't start with the word Test.
Description of Changes
Maybe we should rename them, but for now lets reconfigure pytest to discover them.
Testing
Let's see if they show up in the test log. (Maybe some of them will fail)...
Reviewer Tips
Can anyone think of other ways to check we have got and are running everything?
Did anyone compare the total number of old nose tests and new pytest tests?