Skip to content

Commit

Permalink
Consistency fixes for test_executor_sequential (#566)
Browse files Browse the repository at this point in the history
A few things here:
1. Use a real Job object like the other test. The sequential executor
   doesn't seem to need it to have a 'identifier' property, but other
   executors do and we should be consistent.
2. Move stuff out of the try/finally block for the os.kill thread to
   reduce the likelihood of secondary exceptions.
3. Clear the global job accumulation variable even though we don't use
   check it (which is another conversation entirely).
  • Loading branch information
cottsay authored Aug 17, 2023
1 parent 4c4da72 commit 18ac286
Showing 1 changed file with 17 additions and 7 deletions.
24 changes: 17 additions & 7 deletions test/test_executor_sequential.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,17 @@ def test_sequential():
ran_jobs.clear()


async def job8():
global ran_jobs
await asyncio.sleep(1)
ran_jobs.append('job8')
class Job8(Job):

def __init__(self):
super().__init__(
identifier='job8', dependencies=set(), task=None,
task_context=None)

async def __call__(self, *args, **kwargs):
global ran_jobs
await asyncio.sleep(3)
ran_jobs.append(self.identifier)


@pytest.fixture
Expand All @@ -174,21 +181,24 @@ def test_sequential_keyboard_interrupt(restore_sigint_handler):
args = None
jobs = OrderedDict()
jobs['one'] = Job1()
jobs['aborted'] = job8
jobs['aborted'] = Job8()
jobs['four'] = Job4()

def delayed_sigint():
time.sleep(0.1)
# Note: a real Ctrl-C would signal the whole process group
os.kill(
os.getpid(),
signal.SIGINT if sys.platform != 'win32' else signal.CTRL_C_EVENT)
if sys.platform == 'win32':
os.kill(os.getpid(), signal.CTRL_C_EVENT)

thread = Thread(target=delayed_sigint)
thread.start()
try:
thread.start()
rc = extension.execute(args, jobs)
assert rc == signal.SIGINT
finally:
thread.join()

assert rc == signal.SIGINT
ran_jobs.clear()

0 comments on commit 18ac286

Please sign in to comment.