Skip to content

Commit

Permalink
fix(python): bin script python support, test argument passing (#3762)
Browse files Browse the repository at this point in the history
Corrects multiple issues with running `bin` scripts from python.

These were discovered in the context of this `projen` issue: projen/projen#2103

 - Corrects broken argument marshaling to binary scripts introduced in #3694
 - Exposes exit code and stderr from script execution instead of failing silently

Additionally, adds more robust test coverage of passing arguments to `bin` scripts.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the [Apache 2.0 license].

[Apache 2.0 license]: https://www.apache.org/licenses/LICENSE-2.0
  • Loading branch information
jmalins authored Sep 23, 2022
1 parent 72f2e7c commit 0cf5008
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 22 deletions.
9 changes: 9 additions & 0 deletions .all-contributorsrc
Original file line number Diff line number Diff line change
Expand Up @@ -1466,6 +1466,15 @@
"contributions": [
"bug"
]
},
{
"login": "jmalins",
"name": "Jeff Malins",
"avatar_url": "https://avatars.githubusercontent.com/u/2001356?v=4",
"profile": "https://github.com/jmalins",
"contributions": [
"code"
]
}
],
"repoType": "github",
Expand Down
25 changes: 13 additions & 12 deletions README.md

Large diffs are not rendered by default.

25 changes: 21 additions & 4 deletions packages/@jsii/kernel/src/kernel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2144,10 +2144,27 @@ defineTest('invokeBinScript() return output', (sandbox) => {
script: 'calc',
});

expect(result.stdout).toEqual('Hello World!\n');
expect(result.stderr).toEqual('');
expect(result.status).toEqual(0);
expect(result.signal).toBeNull();
expect(result).toMatchObject<api.InvokeScriptResponse>({
status: 0,
stdout: 'Hello World!\n',
stderr: '',
signal: null,
});
});

defineTest('invokeBinScript() accepts arguments', (sandbox) => {
const result = sandbox.invokeBinScript({
assembly: 'jsii-calc',
script: 'calc',
args: ['arg1', 'arg2'],
});

expect(result).toMatchObject<api.InvokeScriptResponse>({
status: 0,
stdout: 'Hello World!\n arguments: arg1, arg2\n',
stderr: '',
signal: null,
});
});

// =================================================================================================
Expand Down
22 changes: 18 additions & 4 deletions packages/@jsii/python-runtime/src/jsii/_runtime.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import abc
import os
import sys

import attr

from typing import cast, Any, Callable, List, Optional, Mapping, Type, TypeVar
from typing import Sequence, cast, Any, Callable, List, Optional, Mapping, Type, TypeVar

from . import _reference_map
from ._compat import importlib_resources
Expand Down Expand Up @@ -47,10 +48,23 @@ def load(cls, *args, _kernel=kernel, **kwargs) -> "JSIIAssembly":

@classmethod
def invokeBinScript(
cls, pkgname: str, script: str, *args: str, _kernel=kernel
) -> None:
cls,
pkgname: str,
script: str,
args: Optional[Sequence[str]] = None,
_kernel=kernel,
) -> int:
if args is None:
args = []

response = _kernel.invokeBinScript(pkgname, script, args)
print(response.stdout)
if response.stdout:
print(response.stdout)

if response.stderr:
print(response.stderr, file=sys.stderr)

return response.status


class JSIIMeta(_ClassPropertyMeta, type):
Expand Down
38 changes: 38 additions & 0 deletions packages/@jsii/python-runtime/tests/test_invoke_bin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import platform
import subprocess
import sys
import pytest


class TestInvokeBinScript:
@pytest.mark.skipif(
platform.system() == "Windows",
reason="jsii-pacmak does not generate windows scripts",
)
def test_invoke_script(self) -> None:
result = run_script("calc")

assert result.returncode == 0
assert result.stdout == b"Hello World!\n\n"
assert result.stderr == b""

@pytest.mark.skipif(
platform.system() == "Windows",
reason="jsii-pacmak does not generate windows scripts",
)
def test_invoke_script_with_args(self) -> None:
result = run_script("calc", "arg1", "arg2")

assert result.returncode == 0
assert result.stdout == b"Hello World!\n arguments: arg1, arg2\n\n"
assert result.stderr == b""


def run_script(script_name: str, *args: str) -> subprocess.CompletedProcess:
if platform.system() == "Windows":
# currently not used, the calling semantics have not been defined for bin scripts on windows
script_path = f".env\\Scripts\\{script_name}"
return subprocess.run([sys.executable, script_path, *args], capture_output=True)
else:
script_path = f".env/bin/{script_name}"
return subprocess.run([script_path, *args], capture_output=True)
5 changes: 5 additions & 0 deletions packages/jsii-calc/bin/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,8 @@
/* eslint-disable no-console */

console.info('Hello World!');

const args = process.argv.slice(2);
if (args.length > 0) {
console.info(` arguments: ${args.join(', ')}`);
}
3 changes: 2 additions & 1 deletion packages/jsii-pacmak/lib/targets/python.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1792,14 +1792,15 @@ class PythonModule implements PythonType {
code.line();
emitList(
code,
'__jsii_assembly__.invokeBinScript(',
'exit_code = __jsii_assembly__.invokeBinScript(',
[
JSON.stringify(this.assembly.name),
JSON.stringify(name),
'sys.argv[1:]',
],
')',
);
code.line('exit(exit_code)');
code.closeFile(script_file);
scripts.push(script_file.replace(/\\/g, '/'));
}
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 0cf5008

Please sign in to comment.