Fix crash when sys.stdin is None

[emilburzo]

Fixes #7119

---

This happens especially on AWS Lambda.

(I did not add a news entry because it feels like a trivial change)

[chrahunt]

Hi @emilburzo!

Can you please create an issue instead, providing the relevant details and ideally a way to easily reproduce this? As it stands it's very difficult to evaluate this change, and without understanding or tests it would be even harder to prevent a regression in the future.

[pradyunsg]

This was created in #7119.

[pradyunsg]

(I did not add a news entry because it feels like a trivial change)

Could you please add news/7119.bugfix and news/7118.bugfix, both with the same contents:

Fix a crash when ``sys.stdin`` is set to ``None``, such as on AWS Lambda.

[chrahunt]

Can we take the approach mentioned here, pulling the interactivity check into a separate function that can be tested in isolation?

[emilburzo]

Can we take the approach mentioned here, pulling the interactivity check into a separate function that can be tested in isolation?

I'm new to the project code layout, but I can give it a go. :)

I have one uncertainty though: do we want to have the new interactivity check function in the Subversion class?

Or, because it's so generic, would it be better placed somewhere in a "higher" class? (where?)

[chrahunt]

Thanks. :)

I would put it in pip._internal.utils.misc.

[emilburzo]

I extracted the function, feedback on naming things is very welcomed :)

Is testing this feasible?

I guess we could forcefully set sys.stdin to None, but I'm not sure about the impact.

Any thoughts?

[chrahunt]

Hi @emilburzo!

Thanks for the update. is_console_interactive seems like a fine name to me. I think this is definitely testable.

I would add a helper function and two tests in tests/unit/test_utils.py (probably at the bottom).

The function would take a monkeypatch argument and do something like monkeypatch.setattr(sys.stdin, 'isatty', Mock(return_value=True)), this ensures that the behavior is consistent no matter if we're running locally or in CI.

The first test would call the helper function then make sure that is_console_interactive returns True.

The second (maybe test_is_console_interactive_when_stdin_is_None?) would call the helper function, then monkeypatch.setattr(sys, 'stdin', None), then check that is_console_interactive returns False.

That ensures that we're always testing the situation that would succeed except for the other changes we make for the failure test cases - this will help a lot with the other change mentioned in #7042 later.

[chrahunt]

Assuming everything passes, this looks good to me. Thanks for your work on this @emilburzo!

[emilburzo]

and thank you @chrahunt for the hand-holding, it was very helpful!

[pradyunsg]

Hurrah!

Thanks @emilburzo for the PR and @chrahunt for all the help and reviews. :)

@emilburzo -- do let us know if you're interested in contributing further. There's a lot of places in pip where we could use some help! ^>^

[emilburzo]

@pradyunsg no promises, but I'd at least be interested to see what other things you need help with, maybe there's something I can help with.