Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
44 changes: 40 additions & 4 deletions test/commands/exec_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ def queue_test_success_res(sessionId="testSessionId"):
return {"success": True, "sessionId": sessionId}


fake_valid_URL = "something.bar.foo"


def app_config_res():
return {"hostManifest": {"lab": {"workcell": {"url": "something.bar.foo"}}}}
return {"hostManifest": {"lab": {"workcell": {"url": fake_valid_URL}}}}


def mock_api_endpoint():
Expand All @@ -30,7 +33,15 @@ def ap_file(tmpdir_factory):
"""Make a temp autoprotocol file"""
path = tmpdir_factory.mktemp("foo").join("ap.json")
with open(str(path), "w") as f:
f.write("{}") # any valid json works
payload = {
"instructions": [
{"op": "provision", "x_human": True},
{"op": "uncover"},
{"op": "spin"},
{"op": "cover"},
]
}
f.write(json.dumps(payload))
return path


Expand Down Expand Up @@ -94,7 +105,7 @@ def mockpost(*args, **kwargs):
result = cli_test_runner.invoke(
cli, ["exec", str(ap_file), "-a", mock_api_endpoint()]
)
assert result.exit_code == 0
assert result.exit_code != 0
assert "Error: " in result.stderr


Expand Down Expand Up @@ -160,5 +171,30 @@ def test_too_many_workcell_definition_arguments(cli_test_runner, monkeypatch, ap
cli,
["exec", str(ap_file), "-a", mock_api_endpoint(), "-s", "anthing", "-w", "wc0"],
)
assert result.exit_code == 0
assert result.exit_code != 0
assert "Error: --workcell-id, --session-id are mutually exclusive." in result.stderr


def test_invalid_filters(cli_test_runner, monkeypatch, ap_file):
invalid_command = [
"-e",
"10",
"-i",
"10",
"-e",
"2-1",
"-i",
"3-6",
"-e",
"anything",
]
invalid_filters = set(filter(lambda v: not v.startswith("-"), invalid_command))
result = cli_test_runner.invoke(
cli,
["exec", str(ap_file), "-a", mock_api_endpoint()] + invalid_command,
)
assert result.exit_code != 0
assert "Error: invalid filters" in result.stderr
assert "(number of instructions: 4)" in result.stderr
for inv in invalid_filters:
assert inv in result.stderr
21 changes: 21 additions & 0 deletions transcriptic/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,20 @@ def format_cmd(manifest):
help="If set, the time constraints will be considered only a suggestion.",
is_flag=True,
)
@click.option(
"--exclude",
"-e",
metavar="FILTER",
help="Remove those instructions from the payload. e.g.: 0, 0-5, x_human, op:povision, x_human!:false",
multiple=True,
)
@click.option(
"--include",
"-i",
metavar="FILTER",
help="Add those instructions to the payload after the --exclude has been applied.",
multiple=True,
)
@click.option(
"--partition-group-size",
type=click.INT,
Expand All @@ -700,7 +714,9 @@ def format_cmd(manifest):
default=None,
help="The device id to use as a swap space when partitioning.",
)
@click.pass_context
def execute(
ctx,
autoprotocol,
api,
no_redirect,
Expand All @@ -711,6 +727,8 @@ def execute(
schedule_at,
schedule_delay,
time_constraints_are_suggestion,
exclude,
include,
partition_group_size,
partition_horizon,
partitioning_swap_device_id,
Expand All @@ -727,7 +745,10 @@ def execute(
schedule_at,
schedule_delay,
time_constraints_are_suggestion,
exclude,
include,
partition_group_size,
partition_horizon,
partitioning_swap_device_id,
ctx.obj.api.email,
)
75 changes: 62 additions & 13 deletions transcriptic/commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -1476,6 +1476,28 @@ def run_protocol(api, manifest, protocol, inputs, view=False, dye_test=False):
return


def validate_filter(filters, number_of_instructions):
invalid_filters = set()

for arg in filters:
if arg.isdigit():
idx = int(arg)
if 0 > idx or idx >= number_of_instructions:
invalid_filters.add(arg)
elif re.match(r"\d+-\d+", arg):
[s, e] = [int(v) for v in arg.split("-")]
if s > e or number_of_instructions <= e:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You should also check here that s is > 0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought about it, I think '\d+' should enforce that right?
With not 0?

invalid_filters.add(arg)
elif ":" in arg:
tokens = arg.split(":")
if len(tokens) != 2:
invalid_filters.add(arg)
else:
invalid_filters.add(arg)

return invalid_filters


def execute(
autoprotocol,
api,
Expand All @@ -1487,9 +1509,12 @@ def execute(
schedule_at,
schedule_delay,
time_constraints_are_suggestion,
exclude,
include,
partition_group_size,
partition_horizon,
partitioning_swap_device_id,
email,
):
# Clean api end point
if api.startswith("http://"):
Expand All @@ -1510,7 +1535,7 @@ def execute(
"Error: '--schedule-delay' and '--schedule-at' are mutually exclusive.",
err=True,
)
return
sys.exit(1)

# Get the requested scheduling time
if schedule_delay is not None:
Expand All @@ -1520,12 +1545,27 @@ def execute(
payload["scheduleAt"] = schedule_at

# Get the autoprotocol
autoprotocol_str = autoprotocol.read()
try:
payload["autoprotocol"] = json.loads(autoprotocol_str)
autoprotocol_json = json.loads(autoprotocol.read())
payload["autoprotocol"] = autoprotocol_json
except json.decoder.JSONDecodeError as err:
click.echo(f"Error decoding autoprotocol json: {err}", err=True)
return
sys.exit(1)

# validate filters
num_instructions = len(autoprotocol_json["instructions"])
invalid_exclude = validate_filter(exclude, num_instructions)
invalid_include = validate_filter(include, num_instructions)
if len(invalid_exclude) + len(invalid_include) > 0:
click.echo(
f"Error: invalid filters: {','.join(invalid_exclude.union(invalid_include))} (number of instructions: {num_instructions})",
err=True,
)
sys.exit(1)

# update the payload
payload["exclude"] = exclude
payload["include"] = include

# device set resolution
in_use = []
Expand All @@ -1536,7 +1576,7 @@ def execute(
payload["deviceSet"] = device_json
except json.decoder.JSONDecodeError as err:
click.echo(f"Error decoding device set json: {err}", err=True)
return
sys.exit(1)
in_use.append("--device-set")

if workcell_id:
Expand All @@ -1551,7 +1591,7 @@ def execute(

if len(in_use) > 1:
click.echo(f"Error: {', '.join(in_use)} are mutually exclusive.", err=True)
return
sys.exit(1)

if len(in_use) == 0:
payload["workcellIdForDeviceSet"] = "wctest-mcx1"
Expand All @@ -1575,9 +1615,10 @@ def execute(
path_tokens = clean_api.split("/")
if len(path_tokens) != 3:
click.echo(
f"Invalid api target, expects base-url/facility/workcell.", err=True
f"Error: Invalid api target, expects base-url/facility/workcell.",
err=True,
)
return
sys.exit(1)

clean_api = f"http://{clean_api}"
path_base = f"http://{path_tokens[0]}"
Expand All @@ -1598,16 +1639,22 @@ def execute(
]["url"]
else:
click.echo(
f"Error when get frontend node address: {res_json}", err=True
f"Error when getting frontend node address: {res_json}", err=True
)
return
sys.exit(1)
except json.decoder.JSONDecodeError:
click.echo(f"Error when get frontend node address: {res.text}", err=True)
return
click.echo(
f"Error when getting frontend node address: {res.text}", err=True
)
sys.exit(1)

# add sentBy
if email is not None:
payload["sentBy"] = email.split("@")[0]

# POST to workcell
test_run_endpoint = f"http://{frontend_node_address}/testRun"
click.echo(f"Sending request to http://{frontend_node_address}")
click.echo(f"Sending request to {test_run_endpoint}")
res = requests.post(test_run_endpoint, json=payload)
try:
res_json = json.loads(res.text)
Expand All @@ -1622,8 +1669,10 @@ def execute(
f"Dashboard can be seen at: {clean_api}/dashboard",
err=True,
)
sys.exit(1)
except json.decoder.JSONDecodeError:
click.echo(f"Error: {res.text}", err=True)
sys.exit(1)


def parse_json(json_file):
Expand Down