Some issues with latest changes

[billsacks]

I believe there are some issues with the latest version of manage_externals (probably from #172 but I haven't investigated carefully to determine which of the most recent PRs is responsible for these issues).

To reproduce this, I checked out the latest version of https://github.com/escomp/cesm (I also tried this with https://github.com/escomp/ctsm and got similar behavior), replaced manage_externals with the latest version from this repo, and then ran manage_externals/checkout_externals without the -o flag. I am using python 3.10.7 (is it possible that this is a python version-specific issue?).

The first thing I noticed is that running manage_externals/checkout_externals prints:

Found locally installed optional components: ./tools/statistical_ensemble_test/pyCECT, ./libraries/FMS, ./components/ww3dev, ./components/mom

This message was surprising to me, because it suggests that those optional components are present, which they are not.

Next I see the message:

Checking out externals: './tools/statistical_ensemble_test/pyCECT'

And then manage_externals stops without doing anything more. Note that the above-printed external is an optional external that should not have been checked out at all, since I ran without the -o flag. But neither that external nor any others were actually checked out.

I also tried running the unit and system tests – by going into the test directory and running make utest and make stest. All unit tests passed, but two system tests failed:

$ make stest
PYTHONPATH=..: python -m unittest discover --buffer --pattern 'test_sys_*.py'
.....E
Stdout:
Test repository name: test_container_full
Test cmd:
pushd /Users/sacks/cesm_code/manage_externals/test/tmp/test_container_full; MANIC_TEST_BARE_REPO_ROOT=/Users/sacks/cesm_code/manage_externals/test/repos /Users/sacks/cesm_code/manage_externals/checkout_externals --externals externals.cfg
Processing externals description file : externals.cfg (/Users/sacks/cesm_code/manage_externals/test/tmp/test_container_full)
Checking local status of required & optional components: simp_tag, simp_branch, simp_opt, mixed_req,
Found locally installed optional components: ./externals/simp_opt
Checking out externals:
.......E
Stdout:
Test repository name: test_container_simple_optional
Test cmd:
pushd /Users/sacks/cesm_code/manage_externals/test/tmp/test_container_simple_optional; MANIC_TEST_BARE_REPO_ROOT=/Users/sacks/cesm_code/manage_externals/test/repos /Users/sacks/cesm_code/manage_externals/checkout_externals --externals externals.cfg --status
Processing externals description file : externals.cfg (/Users/sacks/cesm_code/manage_externals/test/tmp/test_container_simple_optional)
Checking local status of required & optional components: simp_req, simp_opt,
e-o ./externals/simp_opt
e-  ./externals/simp_req
Test cmd:
pushd /Users/sacks/cesm_code/manage_externals/test/tmp/test_container_simple_optional; MANIC_TEST_BARE_REPO_ROOT=/Users/sacks/cesm_code/manage_externals/test/repos /Users/sacks/cesm_code/manage_externals/checkout_externals --externals externals.cfg
Processing externals description file : externals.cfg (/Users/sacks/cesm_code/manage_externals/test/tmp/test_container_simple_optional)
Checking local status of required & optional components: simp_req, simp_opt,
Found locally installed optional components: ./externals/simp_opt
Checking out externals:
........................
======================================================================
ERROR: test_container_full (test_sys_checkout.TestSysCheckout)
Verify that 'full' container with simple and mixed subrepos
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sacks/cesm_code/manage_externals/test/test_sys_checkout.py", line 1388, in test_container_full
    overall, tree = self.execute_cmd_in_dir(under_test_dir,
  File "/Users/sacks/cesm_code/manage_externals/test/test_sys_checkout.py", line 662, in execute_cmd_in_dir
    overall_status, tree_status = checkout.main(options)
  File "/Users/sacks/cesm_code/manage_externals/manic/checkout.py", line 435, in main
    source_tree.checkout(args.verbose, load_all)
  File "/Users/sacks/cesm_code/manage_externals/manic/sourcetree.py", line 380, in checkout
    load_comps = sorted(tmp_comps, key=lambda comp: self._all_components[comp].get_local_path())
  File "/Users/sacks/cesm_code/manage_externals/manic/sourcetree.py", line 380, in <lambda>
    load_comps = sorted(tmp_comps, key=lambda comp: self._all_components[comp].get_local_path())
KeyError: './externals/simp_opt'

Stdout:
Test repository name: test_container_full
Test cmd:
pushd /Users/sacks/cesm_code/manage_externals/test/tmp/test_container_full; MANIC_TEST_BARE_REPO_ROOT=/Users/sacks/cesm_code/manage_externals/test/repos /Users/sacks/cesm_code/manage_externals/checkout_externals --externals externals.cfg
Processing externals description file : externals.cfg (/Users/sacks/cesm_code/manage_externals/test/tmp/test_container_full)
Checking local status of required & optional components: simp_tag, simp_branch, simp_opt, mixed_req,
Found locally installed optional components: ./externals/simp_opt
Checking out externals:

======================================================================
ERROR: test_container_simple_optional (test_sys_checkout.TestSysCheckout)
Verify that container with an optional simple subrepos
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/sacks/cesm_code/manage_externals/test/test_sys_checkout.py", line 1086, in test_container_simple_optional
    overall, tree = self.execute_cmd_in_dir(under_test_dir,
  File "/Users/sacks/cesm_code/manage_externals/test/test_sys_checkout.py", line 662, in execute_cmd_in_dir
    overall_status, tree_status = checkout.main(options)
  File "/Users/sacks/cesm_code/manage_externals/manic/checkout.py", line 435, in main
    source_tree.checkout(args.verbose, load_all)
  File "/Users/sacks/cesm_code/manage_externals/manic/sourcetree.py", line 380, in checkout
    load_comps = sorted(tmp_comps, key=lambda comp: self._all_components[comp].get_local_path())
  File "/Users/sacks/cesm_code/manage_externals/manic/sourcetree.py", line 380, in <lambda>
    load_comps = sorted(tmp_comps, key=lambda comp: self._all_components[comp].get_local_path())
KeyError: './externals/simp_opt'

Stdout:
Test repository name: test_container_simple_optional
Test cmd:
pushd /Users/sacks/cesm_code/manage_externals/test/tmp/test_container_simple_optional; MANIC_TEST_BARE_REPO_ROOT=/Users/sacks/cesm_code/manage_externals/test/repos /Users/sacks/cesm_code/manage_externals/checkout_externals --externals externals.cfg --status
Processing externals description file : externals.cfg (/Users/sacks/cesm_code/manage_externals/test/tmp/test_container_simple_optional)
Checking local status of required & optional components: simp_req, simp_opt,
e-o ./externals/simp_opt
e-  ./externals/simp_req
Test cmd:
pushd /Users/sacks/cesm_code/manage_externals/test/tmp/test_container_simple_optional; MANIC_TEST_BARE_REPO_ROOT=/Users/sacks/cesm_code/manage_externals/test/repos /Users/sacks/cesm_code/manage_externals/checkout_externals --externals externals.cfg
Processing externals description file : externals.cfg (/Users/sacks/cesm_code/manage_externals/test/tmp/test_container_simple_optional)
Checking local status of required & optional components: simp_req, simp_opt,
Found locally installed optional components: ./externals/simp_opt
Checking out externals:

----------------------------------------------------------------------
Ran 38 tests in 81.814s

FAILED (errors=2)
make: *** [stest] Error 1

I haven't looked into whether these tests are picking up the same issues I found in manual testing, or if they indicate aspects of the tests themselves that need to be changed for the new behavior.

In fixing these system tests, it could be worth introducing a new test that exercises the new behavior, if it isn't too hard (e.g., running checkout_externals with -o when there is at least one optional external, then changing the version of the optional external, then rerunning checkout_externals without -o and making sure that the version is restored to the correct version).

I had intended to do the following manual testing to verify correct operation of the latest manage_externals; after fixing these issues, it would be great to run through this manual testing to verify that things are now working as intended (and/or testing this via a new automated system test)

• Do an initial run of checkout_externals without -o; required components should be present but optional components should not be
• Rerun checkout_externals without -o; optional components still shouldn't be there (this step may be unnecessary)
• Run checkout_externals -o; now the optional components should be present
• In an optional component, do a git checkout of some other version
• Rerun checkout_externals without -o; the changed optional component should be restored to the version listed in Externals.cfg

Ideally, I thought it would also be good to do some testing to verify that optional sub-externals are also treated properly. I'm not sure if there are any that exist out-of-the-box; if not, you could (for example) introduce an optional external in components/clm/Externals_CLM.cfg and then verify that:

• After the initial run of checkout_externals without -o, the optional sub-external is not checked out
• After running checkout_externals -o, the optional sub-external IS checked out
• Similarly to the above manual test, do a git checkout of some other version, then rerun checkout_externals without -o; the optional sub-external should be restored to the listed version

[billsacks]

I have done the manual tests that I suggested above and verified that this is fixed by #175. Thanks @johnpaulalex !

[jedwards4b]

Thank you @billsacks