Skip to content

Commit

Permalink
Tests: Move memory leak tests to main unit test suite
Browse files Browse the repository at this point in the history
The `.github/system_tests/pytest/test_memory_leaks.py` file provided
tests to ensure memory is not being leaked when running processes. These
tests do not require being executed in standalone `pytest` invocation
but can be included in the main unit test suite. Historically, the
separation was required when the main unit test suite was not fully
using `pytest` yet but used a framework based on `unittest`.

With this migration, the last test in the `.github/workflows/tests.sh`
script has been moved and now it merely calls the main test suite. The
CI workflows that called it, now simply directly invoke the command to
run the main test suite and the `tests.sh` script is deleted.
  • Loading branch information
sphuber committed Nov 16, 2023
1 parent ce9acc3 commit 561f93c
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 35 deletions.
3 changes: 2 additions & 1 deletion .github/workflows/ci-code.yml
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,10 @@ jobs:

- name: Run test suite
env:
AIIDA_TEST_PROFILE: test_aiida
AIIDA_WARN_v3: 1
run:
.github/workflows/tests.sh
pytest --cov aiida --verbose tests -m 'not nightly'

- name: Upload coverage report
if: matrix.python-version == 3.9 && github.repository == 'aiidateam/aiida-core'
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/test-install.yml
Original file line number Diff line number Diff line change
Expand Up @@ -252,9 +252,10 @@ jobs:

- name: Run test suite
env:
AIIDA_TEST_PROFILE: test_aiida
AIIDA_WARN_v3: 1
run:
.github/workflows/tests.sh
pytest --cov aiida --verbose tests -m 'not nightly'

- name: Freeze test environment
run: pip freeze | sed '1d' | tee requirements-py-${{ matrix.python-version }}.txt
Expand Down
12 changes: 0 additions & 12 deletions .github/workflows/tests.sh

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -24,48 +24,57 @@
def run_finished_ok(*args, **kwargs):
"""Convenience function to check that run worked fine."""
_, node = run_get_node(*args, **kwargs)
assert node.is_finished_ok, (node.exit_status, node.exit_message)
assert node.is_finished_ok, (node.process_state, node.exit_code)


@pytest.fixture
def check_memory_leaks():
"""Check that no processes remain in memory after the test finishes.
The fixture checks the current processes in memory before running the test which are ignored in the check after the
test. This is to prevent the test failing because some other test in the suite failed to properly cleanup a process.
"""
starting_processes = get_instances(processes.Process, delay=0.2)

yield

# Check that no reference to the process is left in memory. Some delay is necessary in order to allow for all
# callbacks to finish.
process_instances = set(get_instances(processes.Process, delay=0.2)).difference(set(starting_processes))
assert not process_instances, f'Memory leak: process instances remain in memory: {process_instances}'


@pytest.mark.skipif(sys.version_info >= (3, 12), reason='Garbage collecting hangs on Python 3.12')
@pytest.mark.usefixtures('aiida_profile', 'check_memory_leaks')
def test_leak_run_process():
"""Test whether running a dummy process leaks memory."""
inputs = {'a': orm.Int(2), 'b': orm.Str('test')}
run_finished_ok(test_processes.DummyProcess, **inputs)

# check that no reference to the process is left in memory
# some delay is necessary in order to allow for all callbacks to finish
process_instances = get_instances(processes.Process, delay=0.2)
assert not process_instances, f'Memory leak: process instances remain in memory: {process_instances}'


@pytest.mark.skipif(sys.version_info >= (3, 12), reason='Garbage collecting hangs on Python 3.12')
@pytest.mark.usefixtures('aiida_profile', 'check_memory_leaks')
def test_leak_local_calcjob(aiida_local_code_factory):
"""Test whether running a local CalcJob leaks memory."""
inputs = {'x': orm.Int(1), 'y': orm.Int(2), 'code': aiida_local_code_factory('core.arithmetic.add', '/bin/bash')}
run_finished_ok(ArithmeticAddCalculation, **inputs)

# check that no reference to the process is left in memory
# some delay is necessary in order to allow for all callbacks to finish
process_instances = get_instances(processes.Process, delay=0.2)
assert not process_instances, f'Memory leak: process instances remain in memory: {process_instances}'


@pytest.mark.skipif(sys.version_info >= (3, 12), reason='Garbage collecting hangs on Python 3.12')
def test_leak_ssh_calcjob():
@pytest.mark.usefixtures('aiida_profile', 'check_memory_leaks')
def test_leak_ssh_calcjob(aiida_computer_ssh):
"""Test whether running a CalcJob over SSH leaks memory.
Note: This relies on the 'slurm-ssh' computer being set up.
Note: This relies on the localhost configuring an SSH server and allowing to connect to it from localhost.
"""
computer = aiida_computer_ssh()

# Ensure a connection can be opened before launching the calcjob or it will get stuck in the EBM.
with computer.get_transport() as transport:
assert transport.whoami()

code = orm.InstalledCode(
default_calc_job_plugin='core.arithmetic.add',
computer=orm.load_computer('slurm-ssh'),
filepath_executable='/bin/bash'
default_calc_job_plugin='core.arithmetic.add', computer=computer, filepath_executable='/bin/bash'
)
inputs = {'x': orm.Int(1), 'y': orm.Int(2), 'code': code}
run_finished_ok(ArithmeticAddCalculation, **inputs)

# check that no reference to the process is left in memory
# some delay is necessary in order to allow for all callbacks to finish
process_instances = get_instances(processes.Process, delay=0.2)
assert not process_instances, f'Memory leak: process instances remain in memory: {process_instances}'

0 comments on commit 561f93c

Please sign in to comment.