Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Guide to Docs for ARMI Testing Tools #1746

Open
john-science opened this issue Jun 25, 2024 · 16 comments
Open

Add Guide to Docs for ARMI Testing Tools #1746

john-science opened this issue Jun 25, 2024 · 16 comments
Labels
enhancement New feature or request good first issue Good for newcomers low priority Style points and non-features

Comments

@john-science
Copy link
Member

Based on recent questions I've gotten, I think it would be useful to add a guide to the ARMI-specific testing tools to the "dev" section of the ARMI docs. Below is a first-draft that @opotowsky wrote previously.

ARMI Testing Tools

ARMI has many useful tools to streamline tests in the plugins. Included here are some popular ones, or at least ones that should be popular.

If you're trying to write a new unit test, chances are something LIKE it has been done before and you don't need to design it from scratch. Look around ARMI and other plugins for examples of tests.

Testing with runLog

Use Case: Test code that prints to stdout

While there are some other mocking examples in ARMI, none are as heavily used as mockRunLogs. mockRunLogs.BufferLog() is used to capture the runLog output instead of printing it.

In test_comparedb3.py, there is a (simplified here) use case. A portion of the test for _diffSpecialData wants to confirm the below printout has happened, so it uses the getStdout() method to check that the expected printout exists.

getStdout Example
def test_diffSpecialData(self):
    dr = DiffResults(0.01)
    fileName = "test.txt"
    with OutputWriter(fileName) as out:
        with mockRunLogs.BufferLog() as mock:
            #... skip for clarity: create refData & srcData
            _diffSpecialData(refData, srcData, out, dr)
            self.assertEqual(dr.nDiffs(), 0)
            self.assertIn("Special formatting parameters for", mock.getStdout())

There are examples of this throughout ARMI, but sparsely (so far) in plugins. Search for BufferLog or getStdout in the code to find examples.

Self-Cleaning Directories

Use Case: Automatically cleans up tests that create files

from armi.utils.directoryChangers import TemporaryDirectoryChanger

Two main uses of this class in testing:

  1. Standalone test that calls code that creates something (test_operators.py)

def test_snapshotRequest(self, fakeDirList, fakeCopy):
fakeDirList.return_value = ["mccAA.inp"]
with TemporaryDirectoryChanger():
with mockRunLogs.BufferLog() as mock:
self.o.snapshotRequest(0, 1)
self.assertIn("ISOTXS-c0", mock.getStdout())

  1. Setup and teardown of a testing class, where all/most of the tests create something (test_comparedb3.py)

class TestCompareDB3(unittest.TestCase):
"""Tests for the compareDB3 module."""
def setUp(self):
self.td = TemporaryDirectoryChanger()
self.td.__enter__()
def tearDown(self):
self.td.__exit__(None, None, None)
def test_outputWriter(self):
fileName = "test_outputWriter.txt"
with OutputWriter(fileName) as out:
out.writeln("Rubber Baby Buggy Bumpers")
txt = open(fileName, "r").read()
self.assertIn("Rubber", txt)

Note that sometimes it's necessary to give the temporary directory change object a non-default root path:

Include root argument
THIS_DIR = os.path.dirname(__file__)
.
.
.
def test_something():
    with TemporaryDirectoryChanger(root=THIS_DIR): 
        # test something

Load a Test Reactor

Use Case: need a full reactor.
Warning: Expensive, and possibly over-used. Consider whether mocking or BYO components (below) can be used instead.

from armi.reactor.tests.test_reactors import loadTestReactor
Returns an operator and a reactor:

def loadTestReactor(
inputFilePath=TEST_ROOT,
customSettings=None,
inputFileName="armiRun.yaml",
):

So many interfaces and methods require an operator, a reactor, or both. Check out this example in test_fluxReconstructor.py:

And a test in that class using fuel blocks from the loaded reactor (and a FluxReconstructor method from the operator):

Sidebar: speed up tests using test reactor
If using, maybe you can speed up unit tests by reducing the number of rings.

The database interface needs a reactor to load, so the setUp of a class in test_database3.py reduces the size of the reactor ahead of the tests:

Empty Reactors

from armi import tests

  1. getEmptyHexReactor()
  2. getEmptyCartesianReactor()

BYO Blocks & Assemblies

Use Case: likes speedy tests and doesn't need a full reactor

from armi.reactor.tests import test_blocks
buildSimpleFuelBlock() : returns a block with no reactor/assembly lineage

def buildSimpleFuelBlock():
"""Return a simple block containing fuel, clad, duct, and coolant."""

loadtestBlock(): returns a block in an assembly in a reactor

def loadTestBlock(cold=True):
"""Build an annular test block for evaluating unit tests."""

... block.add(stuff) ...

block.setHeight(16.0)
block.autoCreateSpatialGrids()
assembly.add(block)
r.core.add(assembly)
return block

from armi.reactor.tests import test_assemblies

buildTestAssemblies()

def buildTestAssemblies():
"""
Build some assembly objects that will be used in testing.
This builds 2 HexBlocks:
* One with half UZr pins and half UTh pins
* One with all UThZr pins
"""

makeTestAssembly()

def makeTestAssembly(
numBlocks, assemNum, spatialGrid=grids.HexGrid.fromPitch(1.0), r=None
):
coreGrid = r.core.spatialGrid if r is not None else spatialGrid
a = HexAssembly("TestAssem", assemNum=assemNum)
a.spatialGrid = grids.axialUnitGrid(numBlocks)
a.spatialGrid.armiObject = a
a.spatialLocator = coreGrid[2, 2, 0]
return a

Miscellaneous Miscellany

Here are some test files we felt might be useful for people to search to help with test development in plugins.

  • from armi.materials.tests import test_material
  • from armi.nuclearDataIO.cccc.tests import test_labels
  • from armi.nuclearDataIO.tests import test_xsLibraries
  • from armi.physics.neutronics.fissionProductModel.tests import test_lumpedFissionProduct
  • from armi.physics.neutronics.tests import test_crossSectionManager
  • from armi.reactor.blueprints.tests import test_customIsotopics
  • from armi.tests import ArmiTestHelper # compareFilesLineByLine & compareLines
@john-science john-science added enhancement New feature or request help wanted good first issue Good for newcomers low priority Style points and non-features labels Jun 25, 2024
@andfranklin
Copy link

andfranklin commented Jul 19, 2024

RE: Self-Cleaning Directories

A lot of the use cases from TemporaryDirectoryChanger could be covered by using pytest's tmp_path fixture in combination with os.chdir. I'd be willing to bet that simple workarounds could be found for the "non-default root path" use case(s).

Some possible side benefits of this alternative approach:

Aside from historical inertia, is there a strong reason why ARMI would like to continue to maintain TemporaryDirectoryChanger?

@andfranklin
Copy link

andfranklin commented Jul 19, 2024

RE: Load a Test Reactor

I'm working on something for this. It'll be able to cache test reactors for quick reloading between test runs. It'll also be fully compatible with xdist -- NO RACE CONDITIONS! 🎉

It'll be ready for show-time in a week or two. I'm just writing-up documentation and developing realistic benchmark cases.

If it's accepted by ARMI, then loadTestReactor could be greatly simplified. I also have an idea for how to close #1636.

@jakehader
Copy link
Member

Thanks for the help on this @andfranklin!

@jakehader
Copy link
Member

RE: Self-Cleaning Directories

A lot of the use cases from TemporaryDirectoryChanger could be covered by using pytest's tmp_path fixture in combination with os.chdir. I'd be willing to bet that simple workarounds could be found for the "non-default root path" use case(s).

Some possible side benefits of this alternative approach:

Aside from historical inertia, is there a strong reason why ARMI would like to continue to maintain TemporaryDirectoryChanger?

Plugins often use a temporary directory changer outside of testing to run external programs that produce their own temporary files. When the temporary directory is closed after the work is done only some a subset of files are returned.

@opotowsky
Copy link
Member

Aside from historical inertia, is there a strong reason why ARMI would like to continue to maintain TemporaryDirectoryChanger

It's used in the code as well as the tests

@opotowsky
Copy link
Member

RE: Testing with runLog

Would pytest's caplog fixture solve this?

As far as I can tell, runLog is using python's logging module under the hood. It seems like caplog would be sufficient for testing the logs produced by the runLog. If that's true, then a lot of test code be simplified, and some of the helper functions could probably be deleted.

Maybe there's something that I'm not aware of 🤷‍♂️

I think refactoring the logging used for hundreds of tests is probably out of scope for this ticket.

@opotowsky
Copy link
Member

RE: Load a Test Reactor

I'm working on something for this. It'll be able to cache test reactors for quick reloading between test runs. It'll also be fully compatible with xdist -- NO RACE CONDITIONS! 🎉

It'll be ready for show-time in a week or two. I'm just writing-up documentation and developing realistic benchmark cases.

If it's accepted by ARMI, then loadTestReactor could be greatly simplified. I also have an idea for how to close #1636.

This sounds awesome!

@john-science
Copy link
Member Author

RE: Testing with runLog

Would pytest's caplog fixture solve this?

Sorry, what I was saying is we already HAVE a solution for this. mockRunLogs is used in hundreds of tests.

@john-science
Copy link
Member Author

RE: Load a Test Reactor

I'm working on something for this. It'll be able to cache test reactors for quick reloading between test runs. It'll also be fully compatible with xdist -- NO RACE CONDITIONS! 🎉

It'll be ready for show-time in a week or two. I'm just writing-up documentation and developing realistic benchmark cases.

If it's accepted by ARMI, then loadTestReactor could be greatly simplified. I also have an idea for how to close #1636.

This ticket is NOT about features we don't have. If you have a feature request or idea, that would be a different ticket.

The point of this ticket is to write documentation for testing tools we already have in ARMI.

@andfranklin
Copy link

This ticket is NOT about features we don't have. If you have a feature request or idea, that would be a different ticket.

The point of this ticket is to write documentation for testing tools we already have in ARMI.

Understood, just trying to provide a different perspective of what could be. To me it seems like ARMI could relieve some of its maintenance burden if different practices were adopted. Maybe there's a reason why things are the way that they are for reasons that I'm not aware of 🤷‍♂️

@andfranklin
Copy link

I think refactoring the logging used for hundreds of tests is probably out of scope for this ticket.

That's absolutely fair 😆 I wasn't trying to suggest that.

@andfranklin
Copy link

This sounds awesome!

It will be! I'm excited to get feedback from y'all. We'll get to it soon.

@andfranklin
Copy link

Plugins often use a temporary directory changer outside of testing to run external programs that produce their own temporary files. When the temporary directory is closed after the work is done only some a subset of files are returned.

Ah that makes sense, thanks @jakehader and @opotowsky. Sounds like #1780 actually needs to be addressed. I'll see if I can help with that.

@terrapower terrapower deleted a comment from andfranklin Aug 25, 2024
@john-science
Copy link
Member Author

@opotowsky Do you want to take this ticket, or should I? I figure it should be one of us.

@opotowsky
Copy link
Member

@opotowsky Do you want to take this ticket, or should I? I figure it should be one of us.

I can take it but I'm probably full up for work this week. So it would be later. (I'm happy to do it though)

@john-science
Copy link
Member Author

@opotowsky Do you want to take this ticket, or should I? I figure it should be one of us.

I can take it but I'm probably full up for work this week. So it would be later. (I'm happy to do it though)

There's no rush. I wouldn't get to it this week either. If you want it, self-assign. Otherwise, just assign it to me. #shrug NBD either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers low priority Style points and non-features
Projects
None yet
Development

No branches or pull requests

4 participants