Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test with Executorlib #38

Merged
merged 1 commit into from
Oct 1, 2024
Merged

Test with Executorlib #38

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions tests/test_conda_function.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

try:
from conda_subprocess.decorator import conda
from executorlib import Executor
from executorlib.shared.executor import cloudpickle_register
except ImportError:

Expand Down Expand Up @@ -33,3 +34,11 @@ def test_conda_function(self):
number, prefix = add_function(parameter_1=1, parameter_2=2)
self.assertEqual(prefix[-5:], "py312")
self.assertEqual(number, 3)

def test_conda_function_with_executorlib(self):
cloudpickle_register(ind=1)
with Executor(max_cores=1, backend="local", hostname_localhost=True) as exe:
future = exe.submit(add_function, 1, 2)
number, prefix = future.result()
self.assertEqual(prefix[-5:], "py312")
self.assertEqual(number, 3)
Comment on lines +38 to +44
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider enhancing the executor test for more comprehensive coverage.

The new test method test_conda_function_with_executorlib is a good addition to verify the functionality with executorlib. However, consider the following suggestions to make it more robust:

  1. Test with multiple cores: Instead of max_cores=1, consider testing with multiple cores to ensure the function works correctly in a multi-core environment.

  2. Test with different backends: The current test uses backend="local". Consider parameterizing the test to run with different backends if applicable.

  3. Add error handling and timeout: Implement a timeout for future.result() to prevent the test from hanging indefinitely. Also, consider adding error handling to check for specific exceptions that might be thrown by the executor.

  4. Test concurrency: If relevant, consider submitting multiple functions concurrently to test the executor's ability to handle parallel executions.

Here's an example of how you might implement these suggestions:

import pytest
from concurrent.futures import TimeoutError

@pytest.mark.parametrize("max_cores,backend", [(1, "local"), (2, "local"), (1, "remote")])
def test_conda_function_with_executorlib(self, max_cores, backend):
    cloudpickle_register(ind=1)
    with Executor(max_cores=max_cores, backend=backend, hostname_localhost=True) as exe:
        future1 = exe.submit(add_function, 1, 2)
        future2 = exe.submit(add_function, 3, 4)
        
        try:
            number1, prefix1 = future1.result(timeout=10)
            number2, prefix2 = future2.result(timeout=10)
        except TimeoutError:
            self.fail("Executor timed out")
        
        self.assertEqual(prefix1[-5:], "py312")
        self.assertEqual(prefix2[-5:], "py312")
        self.assertEqual(number1, 3)
        self.assertEqual(number2, 7)

This enhanced version tests multiple scenarios and provides better error handling and timeout management.

Loading