clean up

bclaw_runner/tests/test_qc_check.py

@@ -95,14 +95,15 @@ def test_run_all_qc_checks(fake1_cond, fake2_cond, expect, mock_qc_data_files):
     assert result == expect
 
 
-@pytest.mark.parametrize("fake1_cond, fake2_cond, expect_qc_fail", [
-    (None, None, False),  # no checks
-    (["a>1"], ["x<99"], False),  # all pass
-    (["a>1", "b==2"], ["y<98"], True),  # one fail
-    (["b==1"], ["x==99", "y==98"], True),  # multi fail
-    (["a==1", "b==2"], ["x==99", "y==98"], True),  # all fail
+@pytest.mark.parametrize("fake1_cond, fake2_cond, expect", [
+    (None, None, []),  # no checks
+    (["a>1"], ["x<99"], []),  # all pass
+    (["a>1", "b==2"], ["y<98"], ["fake1: b==2"]),  # one fail
+    (["b==1"], ["x==99", "y==98"], ["fake2: x==99", "fake2: y==98"]),  # multi fail
+    (["a==1", "b==2"], ["x==99", "y==98"],
+     ["fake1: a==1", "fake1: b==2", "fake2: x==99", "fake2: y==98"]),  # all fail
 ])
-def test_do_checks(fake1_cond, fake2_cond, expect_qc_fail, mock_qc_data_files, mocker):
+def test_do_checks(fake1_cond, fake2_cond, expect, mock_qc_data_files, mocker):
     mock_abort_execution = mocker.patch("bclaw_runner.src.runner.qc_check.abort_execution")
 
     if fake1_cond is None:
@@ -119,9 +120,9 @@ def test_do_checks(fake1_cond, fake2_cond, expect_qc_fail, mock_qc_data_files, m
             },
         ]
 
-    if expect_qc_fail:
-        # todo: should probably check the contents of qcf.failures
+    if expect:
         with pytest.raises(QCFailure) as qcf:
             do_checks(spec)
+        assert qcf.value.failures == expect
     else:
         do_checks(spec)

cloudformation/bc_core.yaml

@@ -546,6 +546,7 @@ Resources:
       FilterPattern: '{$.function = "gather.*"}'
       LogGroupName: !Ref GatherLambdaLogGroup
 
+  # todo: remove this lambda + associated resources
   QCCheckerLambda:
     Type: AWS::Serverless::Function
     DeletionPolicy: "Retain"

lambda/src/compiler/pkg/batch_resources.py

@@ -173,9 +173,7 @@ def job_definition_rc(step: Step,
                       shell_opt: str) -> Generator[Resource, None, str]:
     logical_name = make_logical_name(f"{step.name}.job.defx")
 
-    # todo: test this
     if isinstance(step.spec["commands"], str):
-        # todo: split on newlines?
         command_list = [step.spec["commands"]]
     else:
         command_list = step.spec["commands"]

lambda/tests/compiler/test_batch_resources.py

@@ -320,6 +320,27 @@ def helper():
         assert resource.spec == expected_rc_spec
 
 
+def test_job_definition_rc_single_line_command(sample_batch_step, compiler_env):
+    command_str = textwrap.dedent("""\
+        commands: |
+            echo one > ${outfile1}
+            echo two > ${outfile2}
+            echo three > ${outfile3}
+    """)
+    command_spec = yaml.safe_load(command_str)
+    nu_sample_batch_step = sample_batch_step.copy()
+    nu_sample_batch_step.update(**command_spec)
+    step = Step("test_step", nu_sample_batch_step, "next_step")
+
+    def helper():
+        _ = yield from job_definition_rc(step, "arn:task:role", "sh")
+
+    for resource in helper():
+        result_spec = json.loads(resource.spec["Properties"]["spec"])
+        command = result_spec["parameters"]["command"]
+        assert command == '["echo one > ${outfile1}\\necho two > ${outfile2}\\necho three > ${outfile3}\\n"]'
+
+
 @pytest.mark.parametrize("spec, expect", [
     ({}, "none"),
     ({"skip_if_output_exists": True}, "output"),
@@ -453,42 +474,6 @@ def helper():
         assert job_def_spec["containerProperties"]["jobRoleArn"] == expected_job_role_arn
 
 
-@pytest.mark.skip(reason="new qc_check")
-def test_handle_batch_with_qc(sample_batch_step, compiler_env):
-    step = Step("step_name", sample_batch_step, "next_step_name")
-
-    step.spec["qc_check"] = {
-        "qc_result_file": "qc_file.json",
-        "stop_early_if": "test_expression",
-        "email_subject": "test subject",
-        "notification": [
-            "test_one@case.com",
-            "test_two@case.com",
-        ],
-    }
-
-    def helper():
-        states = yield from handle_batch(step, {"wf": "params", "versioned": "true"}, False)
-        assert len(states) == 2
-        assert all(isinstance(s, State) for s in states)
-
-        assert states[0].name == "step_name"
-        assert states[0].spec["Resource"] == "arn:aws:states:::batch:submitJob.sync"
-        assert states[0].spec["Parameters"]["JobDefinition"] == "${StepNameJobDefx}"
-        assert states[0].spec["Next"] == "step_name.qc_checker"
-
-        assert states[1].name == "step_name.qc_checker"
-        assert states[1].spec["Next"] == "next_step_name"
-
-    resource_gen = helper()
-    resource_dict = dict(resource_gen)
-
-    expected_keys = ["StepNameJobDefx"]
-    assert list(resource_dict.keys()) == expected_keys
-
-    assert resource_dict["StepNameJobDefx"]["Type"] == "Custom::BatchJobDefinition"
-
-
 def test_handle_batch_auto_inputs(sample_batch_step, compiler_env):
     step = Step("step_name", sample_batch_step, "next_step")
     step.spec["inputs"] = None