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

possible issues with tox support? #163

Closed
jleverenz opened this issue Mar 9, 2017 · 10 comments
Closed

possible issues with tox support? #163

jleverenz opened this issue Mar 9, 2017 · 10 comments

Comments

@jleverenz
Copy link
Contributor

Hi! Just getting started with pyfakefs, it's been a lot of help.

The current tox configuration does not seem to be running properly in my environment, though. Before I submit some possible patches, curious if others are running into similar issues, or the general state of tox support.

Summary of what I'm seeing so far:

  1. Use of 'HOME' in environment variable in tests, when it's not available via the tox configuration, causes test to fail.
  2. Missing tox deps (requirements.txt)
  3. tox and travis version support is different (e.g. travis isn't testing py32, tox is)

Thanks!

@jmcgeheeiv
Copy link
Contributor

Thanks for reaching out, @jleverenz.

Let's let @mrbean-bremen comment first, but I think you have a point there.

@mrbean-bremen
Copy link
Member

Short answer (will have more time in the evening): I have ignored tox.ini completely so far, so I didn't remove py32 support or add py34 - py36 support as in travis - I will have a look at it later.
Dependencies: I don't think it shall be in the requirements.txt, as it is needed only if using tox, which is not mandatory.
'HOME' variable - if you can fix this, it would be great!

@jleverenz
Copy link
Contributor Author

Thanks for the quick feedback. I think I have some changes ready to fix this, happy to share them later today for your review.

@jleverenz
Copy link
Contributor Author

PR created. One clarification, there was no need to update requirements.txt with these changes, it was just missing from the deps configuration of tox, so pytest (for example) wouldn't work in the tox scripts.

@mrbean-bremen
Copy link
Member

Looks very sensible! And you are right, I misunderstood the requirements.txt part, not having used tox so far.
It would be nice if you could add a line a or two about tox support in README.MD.
Also, it would be nice to automatically test tox support with travis, I think there is some kind of integration there - if you know about this, you could advice us.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Mar 9, 2017

Ok, I just installed and ran tox under Windows, and it works surprisingly good out of the box and is completely self-contained.
So forget my previous comment - no need for a README addition, and also it may not make much sense to run it with travis,
I will merge the PR as is and certainly use mox in the future myself, thanks for the good work and the pointer to tox!
EDIT: mox -> tox

mrbean-bremen added a commit that referenced this issue Mar 9, 2017
Fix & update tox support, see #163
@jmcgeheeiv
Copy link
Contributor

Note that mox3 is already a dependency of pyfakefs. I wanted to use the standard unittest.mock, but it had a problem that prevented it from doing what I needed.

I got the impression that although someone was good enough to updatemox3 to Python 3, it seemed that mox3 was close to end of life. I suggest you be aware of this when considering using mox3 more widely in your own work.

@mrbean-bremen
Copy link
Member

mrbean-bremen commented Mar 9, 2017

@jmcgeheeiv : I had heard about tox earlier, but got the impression that it does only work correctly under Linux. I now found out out that it works under Windows like a charm (finds all installed Python versions itself) and speeds up testing different versions a lot. As a bonus, it is integrated with PyCharm, which I use as GUI. So in the future I will make sure that tox.ini is up to date...
And sorry - I accidentally confused tox with mox in my last comment, so I may have confused you, too.

@jmcgeheeiv
Copy link
Contributor

You mentioned mox in your message. Perhaps you meant tox. My comments apply to mox alone.

@mrbean-bremen
Copy link
Member

Right - my bad. I corrected that too late.

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

No branches or pull requests

3 participants