Skip to content

Conversation

@msabramo
Copy link
Contributor

Just add 2 files to .gitignore that get created by the tests.

$ ls -l stderr_logfile stdout_logfile
ls: stderr_logfile: No such file or directory
ls: stdout_logfile: No such file or directory

$ tox -e py26
...
  py26: commands succeeded
  congratulations :)

$ ls -l stderr_logfile stdout_logfile
-rw-r--r--  1 marca  staff  0 Feb 21 17:43 stderr_logfile
-rw-r--r--  1 marca  staff  0 Feb 21 17:43 stdout_logfile

@mnaberez
Copy link
Member

It looks like these two files started being created after #241 was merged. We should probably address the problem that is causing them to be created rather than ignore them.

@msabramo
Copy link
Contributor Author

Playing around a little bit, it seems that these files get generated specifically by some tests in supervisor/tests/test_options.py:

(Note that I'm using the ability to run specific tests from tox that I added in PR #380):

$ find . -name 'std*_logfile'

$ tox -e py26 -- supervisor.tests.test_options
py26 develop-inst-nodeps: /Users/marca/dev/git-repos/supervisor
py26 runtests: commands[0] | nosetests supervisor.tests.test_options
.............................................................................................
----------------------------------------------------------------------
Ran 93 tests in 0.078s

OK
___________________________________________________________________________________ summary ____________________________________________________________________________________
  py26: commands succeeded
  congratulations :)

$ find . -name 'std*_logfile'
./supervisor/stderr_logfile
./supervisor/stdout_logfile

@msabramo
Copy link
Contributor Author

Narrowing it down a bit more:

$ find . -name 'std*_logfile'

$ tox -e py26 -- supervisor.tests.test_options:TestProcessConfig.test_make_dispatchers_stderr_not_redirected
py26 develop-inst-nodeps: /Users/marca/dev/git-repos/supervisor
py26 runtests: commands[0] | nosetests supervisor.tests.test_options:TestProcessConfig.test_make_dispatchers_stderr_not_redirected
.
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK
___________________________________________________________________________________ summary ____________________________________________________________________________________
  py26: commands succeeded
  congratulations :)

$ find . -name 'std*_logfile'
./supervisor/stderr_logfile
./supervisor/stdout_logfile

and:

$ find . -name 'std*_logfile'

$ tox -e py26 -- supervisor.tests.test_options:TestProcessConfig.test_make_dispatchers_stderr_redirected
py26 develop-inst-nodeps: /Users/marca/dev/git-repos/supervisor
py26 runtests: commands[0] | nosetests supervisor.tests.test_options:TestProcessConfig.test_make_dispatchers_stderr_redirected
.
----------------------------------------------------------------------
Ran 1 test in 0.001s

OK
___________________________________________________________________________________ summary ____________________________________________________________________________________
  py26: commands succeeded
  congratulations :)

$ find . -name 'std*_logfile'
./supervisor/stdout_logfile

This last one is interesting because only stdout_logfile gets generated if I run only TestProcessConfig.test_make_dispatchers_stderr_redirected, which is probably a big clue.

I wonder if @jaraco can spot the problem easily, since he authored #241...

@msabramo
Copy link
Contributor Author

It looks like removing the files in tearDown works -- I'll send a PR for that...

@msabramo
Copy link
Contributor Author

OK, #381 seems to prevent these files from being left around.

$ git log -n1
commit f01e17eae77e317221cb90578b23333ebf356b90
Author: Marc Abramowitz <marc@marc-abramowitz.com>
Date:   Sat Feb 22 14:15:02 2014 -0800

    TestProcessConfig.tearDown: Cleanup std{err,out}_logfile files if they
    exist

$ find . -name 'std*_logfile'

$ tox -e py26
...
Ran 758 tests in 3.135s

OK
...
  py26: commands succeeded
  congratulations :)

$ find . -name 'std*_logfile'

@msabramo
Copy link
Contributor Author

Heck, it'll even nuke those files if they already exist!

$ touch stdout_logfile stderr_logfile

$ find . -name 'std*_logfile'
./stderr_logfile
./stdout_logfile

$ tox -e py26
...
  py26: commands succeeded
  congratulations :)

$ find . -name 'std*_logfile'

@mnaberez
Copy link
Member

I think what has happened here is that a change in #241 has caused something in the tests that used to be wired up to a DummyLogger to now be using a real logger. I think that because there are many tests in the suite that use a stdout_logfile filename but they do not actually write a file to disk. You're on the right track isolating what specific tests are causing this. However, I think we should try to fix the condition that is causing them to be written rather than deleting them later.

@msabramo
Copy link
Contributor Author

Aha, good context. Thanks. May or may not have time to look into this today.

@msabramo
Copy link
Contributor Author

This seems to be the exact call stack in the test where the files get created (I observed this by tracing through with pdb++ and seeing when the files appeared):

> /Users/marca/dev/git-repos/supervisor/supervisor/tests/test_options.py(1644)test_make_dispatchers_stderr_not_redirected()
-> dispatchers, pipes = instance.make_dispatchers(process1)

> /Users/marca/dev/git-repos/supervisor/supervisor/options.py(1689)make_dispatchers()
-> dispatchers[stdout_fd] = POutputDispatcher(proc, etype, stdout_fd)

> /Users/marca/dev/git-repos/supervisor/supervisor/dispatchers.py(95)__init__()
-> self._setup_logging(process.config, channel)

> /Users/marca/dev/git-repos/supervisor/supervisor/dispatchers.py(136)__init__()
->      self.mainlog = loggers.handle_file(
            config.options.getLogger(),
            filename=logfile,
            fmt=fmt,
            rotating=not not maxbytes, # optimization
            maxbytes=maxbytes,
            backups=backups)

> /Users/marca/dev/git-repos/supervisor/supervisor/loggers.py(346)handle_file()
-> def handle_file(logger, filename, fmt, rotating=False, maxbytes=0, backups=0):
(Pdb++) args
logger = <supervisor.tests.base.DummyLogger instance at 0x10185cab8>
filename = stdout_logfile
fmt = %(message)s
rotating = True
maxbytes = stdout_logfile_maxbytes
backups = stdout_logfile_backups

> /Users/marca/dev/git-repos/supervisor/supervisor/loggers.py(354)handle_file()
-> handler = RotatingFileHandler(filename, 'a', maxbytes, backups)

@msabramo
Copy link
Contributor Author

OK, see #382 -- this prevents the files from getting created in the first place.

@msabramo
Copy link
Contributor Author

#382 or something like it, where we prevent the files from being created in the first place is a better solution.

@msabramo msabramo closed this Feb 24, 2014
msabramo added a commit to msabramo/supervisor that referenced this pull request Apr 14, 2014
by modifying 3 tests to set instance.std{out,err}_logfile to a filename
in the temp directory by using tempfile.mktemp

This is an alternative solution to the issue mentioned in Supervisor#382 and Supervisor#377.
msabramo added a commit to msabramo/supervisor that referenced this pull request Apr 15, 2014
by modifying 3 tests to use instance.std{out,err}_logfile which are temp
files.

This is an alternative solution to the issue mentioned in Supervisor#382, Supervisor#377,
and Supervisor#417.
msabramo added a commit to msabramo/supervisor that referenced this pull request Apr 15, 2014
by modifying 3 tests to use instance.std{out,err}_logfile which are temp
files.

This is an alternative solution to the issue mentioned in Supervisor#382, Supervisor#377,
and Supervisor#417.

Conflicts:
	supervisor/tests/test_options.py
msabramo added a commit to msabramo/supervisor that referenced this pull request Apr 15, 2014
by modifying 3 tests to use instance.std{out,err}_logfile which are temp
files.

This is an alternative solution to the issue mentioned in Supervisor#382, Supervisor#377,
and Supervisor#417.

Conflicts:
	supervisor/tests/test_options.py
msabramo added a commit to msabramo/supervisor that referenced this pull request Apr 16, 2014
by modifying 3 tests to use instance.std{out,err}_logfile which are temp
files.

This is an alternative solution to the issue mentioned in Supervisor#382, Supervisor#377,
and Supervisor#417.

Conflicts:
	supervisor/tests/test_options.py
msabramo added a commit to msabramo/supervisor that referenced this pull request Apr 16, 2014
by modifying 3 tests to use instance.std{out,err}_logfile which are temp
files.

This is an alternative solution to the issue mentioned in Supervisor#382, Supervisor#377,
and Supervisor#417.

Conflicts:
	supervisor/tests/test_options.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

2 participants