From dc665f3ee16647d53be52ac8c20adc3323f6f60c Mon Sep 17 00:00:00 2001 From: Tom Ward Date: Wed, 25 Sep 2024 17:27:37 +0100 Subject: [PATCH 1/6] Don't require confirmation for full job ids --- jobrunner/cli/kill_job.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/jobrunner/cli/kill_job.py b/jobrunner/cli/kill_job.py index 521f4871..c28e1dd0 100644 --- a/jobrunner/cli/kill_job.py +++ b/jobrunner/cli/kill_job.py @@ -55,20 +55,26 @@ def get_jobs(partial_job_ids): jobs = [] need_confirmation = False for partial_job_id in partial_job_ids: - matches = database.find_where(Job, id__like=f"%{partial_job_id}%") - if len(matches) == 0: - raise RuntimeError(f"No jobs found matching '{partial_job_id}'") - elif len(matches) > 1: - print(f"Multiple jobs found matching '{partial_job_id}':") - for i, job in enumerate(matches, start=1): - print(f" {i}: {job.slug}") - print() - index = int(input("Enter number: ")) - assert 0 < index <= len(matches) - jobs.append(matches[index - 1]) + # look for full matches + full_matches = database.find_where(Job, id=partial_job_id) + if len(full_matches) == 1: + jobs.append(full_matches[0]) else: - need_confirmation = True - jobs.append(matches[0]) + # look for partial matches + partial_matches = database.find_where(Job, id__like=f"%{partial_job_id}%") + if len(partial_matches) == 0: + raise RuntimeError(f"No jobs found matching '{partial_job_id}'") + elif len(partial_matches) > 1: + print(f"Multiple jobs found matching '{partial_job_id}':") + for i, job in enumerate(partial_matches, start=1): + print(f" {i}: {job.slug}") + print() + index = int(input("Enter number: ")) + assert 0 < index <= len(partial_matches) + jobs.append(partial_matches[index - 1]) + else: + need_confirmation = True + jobs.append(partial_matches[0]) if need_confirmation: print("About to kill jobs:") for job in jobs: From c68226696cb5adbcc3b4e83d2f22b7a5ea009e63 Mon Sep 17 00:00:00 2001 From: Tom Ward Date: Wed, 25 Sep 2024 17:28:09 +0100 Subject: [PATCH 2/6] Test the get_jobs function --- tests/cli/test_kill_job.py | 48 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/cli/test_kill_job.py b/tests/cli/test_kill_job.py index 0b5dc52d..d5b8c273 100644 --- a/tests/cli/test_kill_job.py +++ b/tests/cli/test_kill_job.py @@ -9,6 +9,54 @@ from tests.factories import job_factory +def test_get_jobs_partial_id(db, monkeypatch): + # make a fake job + job = job_factory(state=State.RUNNING, status_code=StatusCode.EXECUTING) + + # take the first four characters to make a partial id + partial_job_id = job.id[:4] + partial_job_ids = [partial_job_id] + + monkeypatch.setattr("builtins.input", lambda _: "") + + # search for jobs with our partial id + output_job_ids = kill_job.get_jobs(partial_job_ids) + + assert output_job_ids[0].id == job.id + + +def test_get_jobs_partial_id_quit(db, monkeypatch): + # make a fake job + job = job_factory(state=State.RUNNING, status_code=StatusCode.EXECUTING) + + # take the first four characters to make a partial id + partial_job_id = job.id[:4] + partial_job_ids = [partial_job_id] + + def press_control_c(_): + raise KeyboardInterrupt() + + monkeypatch.setattr("builtins.input", press_control_c) + + # make sure the program is quit + with pytest.raises(KeyboardInterrupt): + kill_job.get_jobs(partial_job_ids) + + +def test_get_jobs_full_id(db): + # make a fake job + job = job_factory(state=State.RUNNING, status_code=StatusCode.EXECUTING) + + # this "partial id" is secretly a full id!! + full_job_id = job.id + full_job_ids = [full_job_id] + + # search for jobs with our partial id + output_job_ids = kill_job.get_jobs(full_job_ids) + + assert output_job_ids[0].id == job.id + + @pytest.mark.needs_docker @pytest.mark.parametrize("cleanup", [False, True]) def test_kill_job(cleanup, tmp_work_dir, db, monkeypatch): From ba3caf0158955dde17ad73a30cecc642c10c61f1 Mon Sep 17 00:00:00 2001 From: Tom Ward Date: Thu, 26 Sep 2024 11:04:33 +0100 Subject: [PATCH 3/6] Add some test stubs --- tests/cli/test_kill_job.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/cli/test_kill_job.py b/tests/cli/test_kill_job.py index d5b8c273..992392f5 100644 --- a/tests/cli/test_kill_job.py +++ b/tests/cli/test_kill_job.py @@ -9,6 +9,25 @@ from tests.factories import job_factory +def test_get_jobs_no_jobs(): + # TODO: expect RuntimeError + pass + + +def test_get_jobs_multiple_matches(): + # TODO: test confirmation + pass + +def test_get_jobs_multiple_params_partial(db, monkeypatch): + # TODO: kill_jobs.get_jobs(["1234", "5678"]) + pass + + +def test_get_jobs_multiple_params_full(db, monkeypatch): + # TODO: kill_jobs.get_jobs(["z6tkp3mjato63dkm", "z6tkp3mjato63dkn"]) + pass + + def test_get_jobs_partial_id(db, monkeypatch): # make a fake job job = job_factory(state=State.RUNNING, status_code=StatusCode.EXECUTING) From ef497e455d89060ca5800ad2ac510696309d09d7 Mon Sep 17 00:00:00 2001 From: Providence-o Date: Thu, 3 Oct 2024 16:17:21 +0100 Subject: [PATCH 4/6] another test stub --- tests/cli/test_kill_job.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/cli/test_kill_job.py b/tests/cli/test_kill_job.py index 992392f5..f69fbfc7 100644 --- a/tests/cli/test_kill_job.py +++ b/tests/cli/test_kill_job.py @@ -11,6 +11,16 @@ def test_get_jobs_no_jobs(): # TODO: expect RuntimeError + + # set a string to use as a partial id + partial_job_id = "1234" + partial_job_ids = [partial_job_id] + + kill_job.get_jobs(partial_job_ids) + + +def test_get_jobs_no_match(): + # TODO: expect RuntimeError pass @@ -18,6 +28,7 @@ def test_get_jobs_multiple_matches(): # TODO: test confirmation pass + def test_get_jobs_multiple_params_partial(db, monkeypatch): # TODO: kill_jobs.get_jobs(["1234", "5678"]) pass From cb0762336c7c9856c0b4d03a5db4bed944841355 Mon Sep 17 00:00:00 2001 From: Providence-o Date: Thu, 3 Oct 2024 16:50:36 +0100 Subject: [PATCH 5/6] implement tests --- tests/cli/test_kill_job.py | 64 +++++++++++++++++++++++++++++--------- 1 file changed, 50 insertions(+), 14 deletions(-) diff --git a/tests/cli/test_kill_job.py b/tests/cli/test_kill_job.py index f69fbfc7..1389985f 100644 --- a/tests/cli/test_kill_job.py +++ b/tests/cli/test_kill_job.py @@ -9,34 +9,70 @@ from tests.factories import job_factory -def test_get_jobs_no_jobs(): - # TODO: expect RuntimeError +def test_get_jobs_no_jobs(db): # set a string to use as a partial id partial_job_id = "1234" partial_job_ids = [partial_job_id] - kill_job.get_jobs(partial_job_ids) + with pytest.raises(RuntimeError): + kill_job.get_jobs(partial_job_ids) + + +def test_get_jobs_no_match(db): + + # make a fake job + job_factory( + state=State.RUNNING, status_code=StatusCode.EXECUTING, id="z6tkp3mjato63dkm" + ) + + partial_job_id = "1234" + partial_job_ids = [partial_job_id] + + with pytest.raises(RuntimeError): + kill_job.get_jobs(partial_job_ids) + + +def test_get_jobs_multiple_matches(db, monkeypatch): + + # make a fake job + job = job_factory( + state=State.RUNNING, status_code=StatusCode.EXECUTING, id="z6tkp3mjato63dkm" + ) + + job_factory( + state=State.RUNNING, status_code=StatusCode.EXECUTING, id="z6tkp3mjato63dkn" + ) + partial_job_id = "kp3mj" + partial_job_ids = [partial_job_id] -def test_get_jobs_no_match(): - # TODO: expect RuntimeError - pass + monkeypatch.setattr("builtins.input", lambda _: "1") + output_job_ids = kill_job.get_jobs(partial_job_ids) -def test_get_jobs_multiple_matches(): - # TODO: test confirmation - pass + assert output_job_ids[0].id == job.id def test_get_jobs_multiple_params_partial(db, monkeypatch): - # TODO: kill_jobs.get_jobs(["1234", "5678"]) - pass + job1 = job_factory( + state=State.RUNNING, status_code=StatusCode.EXECUTING, id="z6tkp3mjato63dkm" + ) + + job2 = job_factory( + state=State.RUNNING, status_code=StatusCode.EXECUTING, id="z6tkp3mjato63dkn" + ) + + partial_job_ids = ["dkm", "dkn"] + + monkeypatch.setattr("builtins.input", lambda _: "") + + # search for jobs with our partial id + output_job_ids = kill_job.get_jobs(partial_job_ids) -def test_get_jobs_multiple_params_full(db, monkeypatch): - # TODO: kill_jobs.get_jobs(["z6tkp3mjato63dkm", "z6tkp3mjato63dkn"]) - pass + assert output_job_ids[0].id == job1.id + assert output_job_ids[1].id == job2.id def test_get_jobs_partial_id(db, monkeypatch): From e553ba042e6a0205f79c00f1b05a00cfe2a7fc06 Mon Sep 17 00:00:00 2001 From: Tom Ward Date: Fri, 4 Oct 2024 17:20:04 +0100 Subject: [PATCH 6/6] Refactor so that we don't use an extra db query * thanks @evansd! --- jobrunner/cli/kill_job.py | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/jobrunner/cli/kill_job.py b/jobrunner/cli/kill_job.py index c28e1dd0..c93ccdd3 100644 --- a/jobrunner/cli/kill_job.py +++ b/jobrunner/cli/kill_job.py @@ -55,26 +55,25 @@ def get_jobs(partial_job_ids): jobs = [] need_confirmation = False for partial_job_id in partial_job_ids: - # look for full matches - full_matches = database.find_where(Job, id=partial_job_id) - if len(full_matches) == 1: - jobs.append(full_matches[0]) + # look for partial matches + partial_matches = database.find_where(Job, id__like=f"%{partial_job_id}%") + if len(partial_matches) == 0: + raise RuntimeError(f"No jobs found matching '{partial_job_id}'") + elif len(partial_matches) > 1: + print(f"Multiple jobs found matching '{partial_job_id}':") + for i, job in enumerate(partial_matches, start=1): + print(f" {i}: {job.slug}") + print() + index = int(input("Enter number: ")) + assert 0 < index <= len(partial_matches) + jobs.append(partial_matches[index - 1]) else: - # look for partial matches - partial_matches = database.find_where(Job, id__like=f"%{partial_job_id}%") - if len(partial_matches) == 0: - raise RuntimeError(f"No jobs found matching '{partial_job_id}'") - elif len(partial_matches) > 1: - print(f"Multiple jobs found matching '{partial_job_id}':") - for i, job in enumerate(partial_matches, start=1): - print(f" {i}: {job.slug}") - print() - index = int(input("Enter number: ")) - assert 0 < index <= len(partial_matches) - jobs.append(partial_matches[index - 1]) - else: + # We only need confirmation if the supplied job ID doesn't exactly + # match the found job + job = partial_matches[0] + if job.id != partial_job_id: need_confirmation = True - jobs.append(partial_matches[0]) + jobs.append(job) if need_confirmation: print("About to kill jobs:") for job in jobs: