Improve automated tests support on Windows #300
Conversation
tests/test_virtualenv.py
Outdated
| def test_create_interpreter(make_one): | ||
| venv, dir_ = make_one(interpreter="python3") | ||
| interpreter = "3.7" if IS_WINDOWS else "python3" |
There was a problem hiding this comment.
Can we make this work regardless of which Python version we're testing with?
|
I'm confused. As far as I know neither the Windows or non-Windows are
testing all Python versions.
I changed python3 to 3.7 as the former can't resolve to an interpreter. Not
to version pin the tests.
How would you like to proceed?
…On Thu, 26 Mar 2020, 02:55 Thea Flowers, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tests/test_virtualenv.py
<#300 (comment)>:
> def test_create_interpreter(make_one):
- venv, dir_ = make_one(interpreter="python3")
+ interpreter = "3.7" if IS_WINDOWS else "python3"
Can we make this work regardless of which Python version we're testing
with?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#300 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK42NJPBCC5TQM56PFLZR3RJK72XANCNFSM4LO6BIVQ>
.
|
|
I would prefer a single session (like tests-3.6) be able to pass if only that interpreter is installed. Does that make sense? |
|
I don't understand what you mean. Are you saying if a user only has Python
3.6 installed, you want all the tests to pass?
When making the change I looked at the rest of the file and see lots of 3.x
specifiers. I don't understand why this one is important and the others are
not. Is this also a problem with `test_condaenv_create_interpreter`?
…On Sun, 29 Mar 2020, 23:54 Thea Flowers, ***@***.***> wrote:
I would prefer a single session (like tests-3.6) be able to pass if *only*
that interpreter is installed. Does that make sense?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#300 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK42NKZSEPNOF3ZTVDIRFTRJ7GT7ANCNFSM4LO6BIVQ>
.
|
|
Yes, the test should either pass or skip. I can't see why this test can't be changed to work with whatever python version the tests are currently running under. |
|
OK, is not passing an interpreter, on just Windows, an acceptable
solution?
That way the interpreter is guaranteed to exist.
…On Mon, 30 Mar 2020, 06:54 Thea Flowers, ***@***.***> wrote:
Yes, the test should either pass or skip. I can't see why this test can't
be changed to work with whatever python version the tests are currently
running under.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#300 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABK42NI5TUAJBXGBC3NEOX3RKAXZXANCNFSM4LO6BIVQ>
.
|
|
sure
On Sun, Mar 29, 2020 at 11:03 PM Peilonrayz <notifications@github.com>
wrote:
… OK, is not passing an interpreter, on just Windows, an acceptable
solution?
That way the interpreter is guaranteed to exist.
On Mon, 30 Mar 2020, 06:54 Thea Flowers, ***@***.***> wrote:
> Yes, the test should either pass or skip. I can't see why this test can't
> be changed to work with whatever python version the tests are currently
> running under.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#300 (comment)>, or
> unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/ABK42NI5TUAJBXGBC3NEOX3RKAXZXANCNFSM4LO6BIVQ
>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#300 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB5I4Y47ZH5VDJ4YVUAFZDRKAY3XANCNFSM4LO6BIVQ>
.
|
|
Howdy, @Peilonrayz. It looks like this PR has gone stale. If you're still interested in getting it merged, let me know and we can get this started again. |
|
Sorry life got busy. I've looked into the problem again and Bottom line; I don't know how to nicely solve the issue we have here. Would the best course of action be to just remove the changes to |
|
Would the best course of action be to just remove the changes to
test_create_interpreter and let the rest of the PR through?
i'm fine with that. :)
…On Sun, Mar 7, 2021 at 5:31 AM Peilonrayz ***@***.***> wrote:
Sorry life got busy. I've looked into the problem again and
make_one(interpreter="python3") fails to resolve to an interpreter for
me. Maybe I have a bug with my setup or nox has a bug making the test fail.
I have gotten make_one(interpreter=None) to work on Windows but I'd just
be adding a potential bug on Linux since the assert dir_.join("bin",
"python3").check() could fail if Python 2 is used. I could use make_one(interpreter=None
if IS_WINDOWS else "python3"), which smells to me.
Bottom line; I don't know how to nicely solve the issue we have here.
Would the best course of action be to just remove the changes to
test_create_interpreter and let the rest of the PR through?
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#300 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAB5I45ZDWGBDW3L2E3WIQ3TCNIZZANCNFSM4LO6BIVQ>
.
|
|
Ok! I've undone the changes to |
|
I re-opened it. Looks like you may need to |
|
Thank you for reopening my PR. FWIW I edited the file on GitHub and when I got to the issue today the commit had appeared here. I had to merge some changes, for the most part I took your changes. Where I kept |
|
Thank you!! |
This fixes the majority of #298. Most of the changes should be self-explanatory.
However, I have changed coverage to combine the output as a 3.8 specific coverage bug was causing
coverto fail.I have not fixed Docs.3 as I'm unsure how I should fix it, this means
docsis still failing.