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

Support for running setup.py nosetests #7

Closed
wants to merge 5 commits into from

Conversation

mdprewitt
Copy link
Contributor

The nosetest plugin wasn't working when running under setup.py. It would fail because the subprocesses would be run as setup.py processes instead of nosetests processes. The output from setup.py was confusing nosetests. So, when running under setup.py, nosetests needs to make sure the sub-process command is nosetests... instead of setup.py nosetests...

I also had dependency problems with nose==dev in setup.py. Getting the following error:

python setup.py develop
...
No local packages or download links found for nose==dev,>=0.1.0
error: Could not find suitable distribution for Requirement.parse('nose==dev,>=0.1.0')

Marc Prewitt added 2 commits February 12, 2015 12:46
Current versions of distutils interpret "nose>=0.1.0, ==dev" as an AND
requirement.  Don't think that it should be a requirement to be runnning
a development version.  Maybe >=dev would be better?
- When run via `setup.py nosetests` strip off setup.py from argv for running
  sub-process.  If this is not done, we end up running another
  `setup.py nosetests ...`.  The output of this interferes with the expected
  output from the test reporter.
- Add try/except for runner to display error from sub-process
# Getting cwd inside SubprocessTestProxy.__call__ is too late - it is
# already changed by nose
self._cwd = os.getcwd()

@staticmethod
def _get_nose_whitelisted_argv():
def _get_nose_whitelisted_argv(nosearg=0):
Copy link
Owner

Choose a reason for hiding this comment

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

Could you rename this new argument to something with more meaning in this function - perhaps "offset"? I'd also like to see the +1 added to the argument before passing it in, rather than doing it multiple times inside of here.

@dmccombs
Copy link
Owner

Thanks for your pull request! This all looks reasonable and useful. If you can make the minor changes from my comments I'll test this out and merge it.

@mdprewitt
Copy link
Contributor Author

Sounds good. I've made the changes. Let me know if you want me to squash before you merge.

stderr=subprocess.STDOUT,
shell=useshell,
)
except OSError, e:
Copy link
Owner

Choose a reason for hiding this comment

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

Just tried this with Python 3 and noticed this is Python 2 only code - it should instead be except OSError as e:.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed this as well as the # in comment. How are you testing? I’ve tried setup.py nosetests and get lots of problems.

On Feb 17, 2015, at 2:45 PM, Dan McCombs [email protected] wrote:

In nosepipe.py #7 (comment):

@@ -147,12 +147,16 @@ def call(self, result):
useshell = True

     self.logger.debug("Executing %s", " ".join(argv))
  •    popen = subprocess.Popen(argv,
    
  •                             cwd=self._cwd,
    
  •                             stdout=subprocess.PIPE,
    
  •                             stderr=subprocess.STDOUT,
    
  •                             shell=useshell,
    
  •                             )
    
  •    try:
    
  •        popen = subprocess.Popen(argv,
    
  •                                 cwd=self._cwd,
    
  •                                 stdout=subprocess.PIPE,
    
  •                                 stderr=subprocess.STDOUT,
    
  •                                 shell=useshell,
    
  •                                 )
    
  •    except OSError, e:
    
    Just tried this with Python 3 and noticed this is Python 2 only code - it should instead be except OSError as e:.


Reply to this email directly or view it on GitHub https://github.com/dmccombs/nosepipe/pull/7/files#r24846994.

Copy link
Owner

Choose a reason for hiding this comment

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

I ran into this just running python3 ./setup.py install

Copy link
Owner

Choose a reason for hiding this comment

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

I should note, after installing this I just ran through my normal tests with nosetests to make sure nothing was broken with this change. I don't normally run via setup.py. Does this now fix your issue without problems? I'm confused by your last comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm running with nosetests. I've setup a travis job to show you the failure I'm getting here:

https://travis-ci.org/mdprewitt/nosepipe/jobs/51232640

I'm also messing around with seeing how I can fix this here:

mdprewitt#1

@mdprewitt
Copy link
Contributor Author

Closing and moving to #8

@mdprewitt mdprewitt closed this Feb 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants