Skip to content

Add support for exec subcommand #2142

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 7, 2021
Merged

Add support for exec subcommand #2142

merged 3 commits into from
Aug 7, 2021

Conversation

gaborbernat
Copy link
Member

@gaborbernat gaborbernat commented Aug 7, 2021

Run arbitrary commands within your tox environment. Don't abuse it.

Resolves #1790

Run arbitrary commands within your tox environment. Don't abuse it.

Signed-off-by: Bernát Gábor <[email protected]>
@codecov
Copy link

codecov bot commented Aug 7, 2021

Codecov Report

Merging #2142 (1a54692) into rewrite (62976a9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           rewrite    #2142   +/-   ##
========================================
  Coverage    99.85%   99.85%           
========================================
  Files          155      157    +2     
  Lines         8693     8744   +51     
  Branches       918      921    +3     
========================================
+ Hits          8680     8731   +51     
  Misses           3        3           
  Partials        10       10           
Flag Coverage Δ
tests 99.85% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/tox/plugin/manager.py 100.00% <100.00%> (ø)
src/tox/session/cmd/devenv.py 100.00% <100.00%> (ø)
src/tox/session/cmd/exec_.py 100.00% <100.00%> (ø)
src/tox/session/common.py 100.00% <100.00%> (ø)
tests/config/cli/conftest.py 100.00% <100.00%> (ø)
tests/session/cmd/test_exec_.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 62976a9...1a54692. Read the comment docs.

Copy link
Member

@jugmac00 jugmac00 left a comment

Choose a reason for hiding this comment

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

While there are several potentially dangerous configuration options (e.g. delete directories) already present in tox, this one is special - let's hope for the best 👍

@asottile Any opinion on this one?

I am not super happy with the autogenerated documentation, as at least I could not see that the command needs to be passed in after a double dash --.
Screenshot from 2021-08-07 11-19-43

Is there anything we could do with the autogenerated docs or maybe should we add a FAQ entry? Although - technically it is not a frequently asked question :-)

@jugmac00 jugmac00 requested a review from asottile August 7, 2021 09:25
@gaborbernat
Copy link
Member Author

I am not super happy with the autogenerated documentation, as at least I could not see that the command needs to be passed in after a double dash --.

It's documented in the epilogue of the sub-command (see tox4 e --help), if it's not rendering means we need to fix that in our documentation generation.

Signed-off-by: Bernát Gábor <[email protected]>
@jugmac00
Copy link
Member

jugmac00 commented Aug 7, 2021

I am not super happy with the autogenerated documentation, as at least I could not see that the command needs to be passed in after a double dash --.

It's documented in the epilogue of the sub-command (see tox4 e --help), if it's not rendering means we need to fix that in our documentation generation.

Thanks - tox4 exec --help indeed renders the example - the documentation does not - yet.

@gaborbernat
Copy link
Member Author

Created tox-dev/sphinx-argparse-cli#20 to address that.

@gaborbernat
Copy link
Member Author

While there are several potentially dangerous configuration options (e.g. delete directories) already present in tox, this one is special - let's hope for the best

I don't think it's overly dangerous. Users can already invoke commands within the tox environments (as in .tox/py39/bin/pip install x). The only thing this adds is convenience to easier invocation for that, and allowing you to take advantage of tox environment isolation and altering. And we're all consenting adults here at the end of the day, not?

@gaborbernat gaborbernat merged commit 174fd08 into tox-dev:rewrite Aug 7, 2021
@gaborbernat gaborbernat deleted the 1790 branch August 7, 2021 22:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants