Skip to content

update most tests, don't use stdout in ipsframework, fix a few bugs - #257

Open
Lance-Drane wants to merge 3 commits into
0.9.0-prereleasefrom
fix_tests
Open

update most tests, don't use stdout in ipsframework, fix a few bugs#257
Lance-Drane wants to merge 3 commits into
0.9.0-prereleasefrom
fix_tests

Conversation

@Lance-Drane

Copy link
Copy Markdown

This mostly fixes tests, except for a few: test_trace.py is timing-dependent and test_basic_serial fails under pytest's forked multiprocessing (loses venv resolution). Will address these later

This also removes several redundant print statements, and swaps any remaining STDOUT outputs to STDERR EXCEPT for the logfile property in the launch function. This is useful for general "force a flush after call, whether or not STDOUT is a TTY" purposes, and will be useful in the future for setting up E2E tests.

Renames

  • fixed a few additional renames in the tests

Bugs fixed

The AI caught a few bugs here:

  • task_manager.py: mpirun --display ALLOCATION,MAP-DEVEL,BINDINGS was wrongly added for plain single-node mpirun (should only apply to DVM/prun)
  • services.py - submit_dask_tasks() computed return len(self.futures) after resetting self.futures = [], always returning 0; and get_dask_finished_tasks_status() discarded blocking-mode results and got mis-routed after _shutdown_dask() reset dask_pool=False — both fixed by caching results in self.dask_results
  • services.py - _process_dask_event crashed on the return_value key Dask injects into callable-task events; now stripped like the existing worker key

Quick code minimizing

Only made changes here which didn't change the APIs in the services class

  • remove all Pyro code, it's not needed
  • replace most of ipsutil.which() implementation with shutil.which() (could probably remove this entirely if we don't care about the unique API)
  • simplify EventManager implementation
  • replace most of debug.py with the stdlib logger, keeping the IPSES_DEBUG environment variable for now. We may want to remove this file eventually.

The AI also recommended nuking the RUS section (especially the vendored fork of the configobj dependency when we are including this in our own dependencies) and to consider shrinking cca_es_spec.py .

Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
1) don't use hand-rolled env gated file logger --- 2) remove Pyro code --- 3) use shutil.which instead of handrolled 'which' --- 4) simplify EventManager by using lazy-set attributes instead of objcache dict

Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
Signed-off-by: Lance-Drane <Lance-Drane@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant