Skip to content
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

Add interactive property to session. #196

Merged
merged 10 commits into from
May 29, 2019
Merged

Add interactive property to session. #196

merged 10 commits into from
May 29, 2019

Conversation

AGeekInside
Copy link
Contributor

I took a stab at adding the interactive attribute to session. The tests are present, but I think it'll need some testing in a non-interactive pipeline to confirm it works. The unit tests in test_sessions feel a bit clunky with the mocks, so suggestions on making that cleaner would be great.

I need to sort out how to update the docs as well to include the new attribute.

If this looks reasonable to folks, I can take a look at adding the '--non-interactive' flag.

Copy link
Collaborator

@theacodes theacodes left a comment

Choose a reason for hiding this comment

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

Thank you for doing this, I just have a few comments!

nox/sessions.py Outdated
@@ -115,6 +116,11 @@ def bin(self):
"""The bin directory for the virtualenv."""
return self._runner.venv.bin

@property
def interactive(self):
"""Boolean showing if the session was ran in an interactive sessions."""
Copy link
Collaborator

@theacodes theacodes May 22, 2019

Choose a reason for hiding this comment

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

Use active voice, please: True if the Nox is being run in an interactive session or False otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just changed it in the latest commit

@@ -83,6 +83,21 @@ def test_properties(self):
assert session.bin is runner.venv.bin
assert session.python is runner.func.python

def test_interactive(self):
with mock.patch("nox.sessions.sys") as mock_sys:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to just patch sys.stdout.istty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep. I just pushed a commit with the change.

@theacodes
Copy link
Collaborator

This is wonderful, @AGeekInside. I'm going to merge #187 first and add a --non-interactive option to negate this before merging. Thank you so much!

@theacodes theacodes self-assigned this May 23, 2019
@theacodes theacodes changed the title WIP: Changes to add interactive attribute to session. [Awaiting 187] Add interactive property to session. May 23, 2019
Copy link
Collaborator

@stsewd stsewd left a comment

Choose a reason for hiding this comment

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

👍 I left some small suggestions

nox/sessions.py Outdated Show resolved Hide resolved
tests/test_sessions.py Outdated Show resolved Hide resolved
tests/test_sessions.py Outdated Show resolved Hide resolved
@theacodes theacodes changed the title [Awaiting 187] Add interactive property to session. Add interactive property to session. May 29, 2019
@theacodes theacodes merged commit d49c148 into wntrblm:master May 29, 2019
@theacodes
Copy link
Collaborator

Thank you, @AGeekInside!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants