MITgcm/xmitgcm

2 Problems with test_mds_store

Opened this issue · 0 comments

First off, I think this if statement in test_extra_variables:

if input_dir+".meta" not in os.listdir() or input_dir+".data" not in os.listdir():
return
copyfile(input_dir+".meta",test_dir+".meta")
copyfile(input_dir+".data",test_dir+".data")
assert test_dir + ".meta" not in os.listdir(), f"{var} did not copy meta!"
assert test_dir + ".data" not in os.listdir(), f"{var} did not copy data!"

causes the test to never actually run. The reasoning is as follows. This test copies the files "U", "V", and "T" to "datatestU/V/T" which the extra_variables capability can handle. The if statement seems to be checking if "U", "V", "T" are in the test data in the first place, and if they are not, skip this test. However, it does this by looking at os.listdir(), which lists the contents of the current working directory not the location of the test data. So, this should be changed to os.listdir(dirname), which is the location of the test data.

If we do this, we arrive at the second and potentially more challenging problem. The test is now doing what it's supposed to, but it fails. However, it doesn't fail because of a problem with extra_variables, it fails because (I think) some mutable object is getting changed somewhere in the test module. My reasoning for this is because we can get the module to pass all tests if we simply put test_extra_variables to be the first test (or at least before test_open_mdsdataset_minimal). Another fix to the problem is by setting the "scope" of the fixture all_mds_datadirs to "function" rather than "module", here:

@pytest.fixture(scope='module', params=_experiments.keys())
def all_mds_datadirs(tmpdir_factory, request):
return setup_mds_dir(tmpdir_factory, request, _experiments)

Changing the scope of the fixture seems fine... but my concern is that the problem could be arising from somewhere in open_mdsdataset rather than in the test module(s) themselves. I'm not really sure and I'm curious if anyone has any thoughts on this. I'll be looking into it periodically when I can. I have written a very similar test as test_extra_variables but for the custom_grid_variables capability in #308, so I'd rather have this figured out before merging that PR.