ucphhpc / migrid-sync

MiGrid workspace where master branch is kept strictly in sync with SF upstream svn repo. Any development or experiments should use a branch. You probably want to fork your own clone or work e.g. on the edge branch if you wish to contribute.
GNU General Public License v2.0
3 stars 4 forks source link

Bringup of the configuration object cross-version and coarse coverage. #77

Closed albu-diku closed 1 month ago

albu-diku commented 2 months ago

Arrange environment specific test configurations such that codepaths that depend upon it (i.e call the get_configuration_object() function) can be used under test. Do this by providing an explicit config making facility as part of the enviromnent helpers which operates based on an environment name e.g. "local" and "test".

For cross-version support the test configuration must be adapted for differences in paths (specifically Python 2 where execution occurs within a container and thus container-relative paths are necessary). Support a --python2 command line argument in makeconfig which causes the paths adjusted as necessary.

Add a wrapper to utilise the pre-existing testcore unit tests as part of the automatde test suite. This is done in order to exercise some codepaths that depend on a valid configuration object. Note though that this approach can likely be used as an examplar of the wrapping necessary to expose existing tests.

jonasbardino commented 1 month ago

Looks good with one pending question and some pending style polish, including a bit overlap with previous PR comments.

A make test from distclean state gives me a single error:

running with MIG_ENV='local' under python 3

...........sss.....sss..........F..
======================================================================
FAIL: test_existing_main (test_mig_unittest_testcore.MigUnittestTestcore)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jonas/ku/migrid-sync/migrid-pr76/mig/unittest/testcore.py", line 56, in _assert_local_config
    configdir_stat = os.stat(_LOCAL_CONF_DIR)
FileNotFoundError: [Errno 2] No such file or directory: '/home/jonas/ku/migrid-sync/migrid-pr76/envhelp/output/testconfs-py3'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/jonas/ku/migrid-sync/migrid-pr76/tests/test_mig_unittest_testcore.py", line 51, in test_existing_main
    testcore_main(_exit=raise_on_error_exit)
  File "/home/jonas/ku/migrid-sync/migrid-pr76/mig/unittest/testcore.py", line 91, in main
    config = _assert_local_config()
  File "/home/jonas/ku/migrid-sync/migrid-pr76/mig/unittest/testcore.py", line 65, in _assert_local_config
    raise AssertionError('local configuration invalid or missing: %s' % (str(exc),))
AssertionError: local configuration invalid or missing: [Errno 2] No such file or directory: '/home/jonas/ku/migrid-sync/migrid-pr76/envhelp/output/testconfs-py3'

----------------------------------------------------------------------
Ran 35 tests in 0.213s

FAILED (failures=1, skipped=6)
make: *** [Makefile:44: test] Error 1

Just adding the dir and retrying results in further errors (`KeyError: 'GLOBAL' etc.), so I better leave that to you.

albu-diku commented 1 month ago

Looks good with one pending question and some pending style polish, including a bit overlap with previous PR comments.

A make test from distclean state gives me a single error:

running with MIG_ENV='local' under python 3

...........sss.....sss..........F..
======================================================================
FAIL: test_existing_main (test_mig_unittest_testcore.MigUnittestTestcore)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jonas/ku/migrid-sync/migrid-pr76/mig/unittest/testcore.py", line 56, in _assert_local_config
    configdir_stat = os.stat(_LOCAL_CONF_DIR)
FileNotFoundError: [Errno 2] No such file or directory: '/home/jonas/ku/migrid-sync/migrid-pr76/envhelp/output/testconfs-py3'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/jonas/ku/migrid-sync/migrid-pr76/tests/test_mig_unittest_testcore.py", line 51, in test_existing_main
    testcore_main(_exit=raise_on_error_exit)
  File "/home/jonas/ku/migrid-sync/migrid-pr76/mig/unittest/testcore.py", line 91, in main
    config = _assert_local_config()
  File "/home/jonas/ku/migrid-sync/migrid-pr76/mig/unittest/testcore.py", line 65, in _assert_local_config
    raise AssertionError('local configuration invalid or missing: %s' % (str(exc),))
AssertionError: local configuration invalid or missing: [Errno 2] No such file or directory: '/home/jonas/ku/migrid-sync/migrid-pr76/envhelp/output/testconfs-py3'

----------------------------------------------------------------------
Ran 35 tests in 0.213s

FAILED (failures=1, skipped=6)
make: *** [Makefile:44: test] Error 1

Just adding the dir and retrying results in further errors (`KeyError: 'GLOBAL' etc.), so I better leave that to you.

The testconfigs are generated by dependencies within the Makefile so almost sounds like something didn't get rebuilt right post distclean. Will look into that.

jonasbardino commented 1 month ago

I'm sort of ready to merge if you can agree to my three recent comments - I can easily adjust that as part of the merge. I basically just want to make sure that we don't end up with wrong paths in production MiGserver.conf and that I can run python3 ./mig/install/generateconfs.py without user/group args, say in my local home, without getting the old KeyError: "getpwnam(): name not found: 'mig' crash.

It looks like the make test issue mentioned above was fixed during your follow-up commits. So I assume I can just resolve that conversation, right?

jonasbardino commented 1 month ago

I've pushed my adjustments to fold in PR75 details as agreed off-list - along with a minor consistency change to align generated output dir variable with the result in the backend helper. Please have a look and let me now if I can merge @albu-diku .

jonasbardino commented 1 month ago

Manually merged through svn with another round of minor adjustments pushed here first. There are a couple of unresolved bits above but make test and basic conf generation seems to fundamentally work.