Skip to content

Conversation

blueyed
Copy link
Contributor

@blueyed blueyed commented Jan 10, 2020

Waiting on davidhalter/jedi#1472.
Blocked by #6435.

@blueyed blueyed changed the title pytester: typing for spawn/spawn_pytest [WIP] pytester: typing for spawn/spawn_pytest Jan 10, 2020
Copy link
Member

@bluetech bluetech left a comment

Choose a reason for hiding this comment

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

LGTM.

BTW, the code in spawn_pytest is pretty scary 😨 , in that it looks open to shell-injection. It would be better to use the args format instead of joining the arguments to a string. That's not related to this PR though, and maybe the input is always good - I didn't check.

        invoke = " ".join(map(str, self._getpytestargs()))
        cmd = "{} --basetemp={} {}".format(invoke, basetemp, string)
        return self.spawn(cmd, expect_timeout=expect_timeout)

@blueyed
Copy link
Contributor Author

blueyed commented Jan 12, 2020

Yeah, I've made it using *args in my fork already - also to have it in line with the other APIs.
IIRC pexpect splits it itself for a string, and handles cmd, *args also.

I'll rebase this after #6435.

@blueyed blueyed changed the title [WIP] pytester: typing for spawn/spawn_pytest pytester: typing for spawn/spawn_pytest Jan 17, 2020
@blueyed blueyed changed the base branch from features to master January 17, 2020 04:58
@blueyed blueyed force-pushed the pytester-typing-spawn branch from 98665f0 to 3694415 Compare January 17, 2020 04:58
@blueyed
Copy link
Contributor Author

blueyed commented Jan 17, 2020

Rebased on master.

@blueyed blueyed merged commit 19f66cb into pytest-dev:master Jan 17, 2020
@blueyed blueyed deleted the pytester-typing-spawn branch January 17, 2020 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants