Skip to content

Conversation

@blueyed
Copy link
Contributor

@blueyed blueyed commented Mar 22, 2019

No description provided.

@blueyed blueyed force-pushed the flaky-pytest_make_collect_report branch from 541d626 to 12e5044 Compare March 22, 2019 15:00
blueyed added 2 commits March 22, 2019 16:38
This reverts commit 12e5044.
This skips all jobs without coverage reporting if
"ci-skip-without-coverage" is found in the commit message.

This is meant for quicker results on PRs - currently only 5 jobs are run
on Travis then, instead of 17.
@nicoddemus nicoddemus mentioned this pull request Mar 22, 2019
@blueyed blueyed force-pushed the flaky-pytest_make_collect_report branch from feb7889 to cee7b0f Compare March 22, 2019 15:59
blueyed added 2 commits March 22, 2019 17:29
ci-skip-without-coverage
@blueyed blueyed force-pushed the flaky-pytest_make_collect_report branch from 70ba3a1 to 503fe39 Compare March 22, 2019 16:53
blueyed added 2 commits March 23, 2019 09:23
Maybe fixes/helps with:

https://pytest-dev.visualstudio.com/pytest/_build/results?buildId=549&view=logs&jobId=b93ccf05-0a7e-58c4-578e-33859c8603c3&taskId=d709c56b-0a9a-5e23-1f1b-6f5848010da1&lineStart=63&lineEnd=64&colStart=1&colEnd=1

  > Exception ignored in: <_io.FileIO name='…\\\\pytest-0\\\\popen-gw1\\\\test_root_logger_affected0\\\\pytest.log' mode='wb' closefd=True>
  > ResourceWarning: unclosed file <_io.TextIOWrapper name='…\\\\pytest-0\\\\popen-gw1\\\\test_root_logger_affected0\\\\pytest.log' mode='w' encoding='UTF-8'>

Analysis:

`test_collect_functools_partial` fails (on purpose via `sys.exit`),
because during its collection this shows up on its stderr (via
`pytest_make_collect_report`).

BUT, the exception and ResourceWarning are referring to the file "pytest.log"
from another test (`test_root_logger_affected`, based on the path).

In another job the same file from this test is involved, but fails
another test then (TestRequestBasic.test_request_addfinalizer).
https://pytest-dev.visualstudio.com/pytest/_build/results?buildId=549&view=logs&jobId=eb23edc9-fa61-53da-8734-34b65fb77a8c&taskId=29ab628b-f8ff-5db3-44d6-286eec35d862&lineStart=65&lineEnd=66&colStart=1&colEnd=1

So another test is getting the ResourceWarning in its stderr while
collecting modules.

This appears to be related to xdist/parallelism.

On Travis TestRequestBasic.test_request_attributes_method caught it [1],
also via/for "gw0/test_root_logger_affected0/pytest.log".

1: https://travis-ci.org/pytest-dev/pytest/jobs/510010086#L785
@blueyed blueyed mentioned this pull request Mar 23, 2019
2 tasks
@blueyed
Copy link
Contributor Author

blueyed commented Mar 23, 2019

I am a bit surprised that the del (appears) to work, since with the following code it triggers it:

import logging

# f = open("/tmp/foo", "a")

log_file_handler = logging.FileHandler("temp.log", mode="w", encoding="UTF-8")

root_logger = logging.getLogger()
root_logger.addHandler(log_file_handler)
root_logger.removeHandler(log_file_handler)
root_logger.error("error")

del log_file_handler

(run with python -Werror::ResourceWarning t.py)

logging itself handles an internal list (using weakrefs), which normally
appears to hold until the logger is there (it has it in self.handlers).
But since we're removing it this reference gets lost.

Anyway, I think this gets triggered by some other test using "-W error" in another thread, while this test is running (xdist)
(test_as_errors, https://github.com/blueyed/pytest/blob/14ee268d925e375a1fc032b96262558e3fb0864b/testing/test_warnings.py#L92).

  1. I think the test_as_errors test should be run in a subprocess, but maybe we should use some locking in our own logging code?! (e.g. via logging's existing locks?).
  2. the code should be covered reliably (see test_collect_capturing: cover captured stderr #4989).

@blueyed
Copy link
Contributor Author

blueyed commented Mar 24, 2019

Closing in favor of #4988 and #4989.

@blueyed blueyed closed this Mar 24, 2019
@blueyed blueyed deleted the flaky-pytest_make_collect_report branch March 24, 2019 08:55
blueyed added a commit to blueyed/pytest that referenced this pull request Mar 24, 2019
blueyed added a commit to blueyed/pytest that referenced this pull request Mar 26, 2019
While it should be closed in logging's shutdown [1], the following would
still issue a ResourceWarning:

```
import logging

log_file_handler = logging.FileHandler("temp.log", mode="w", encoding="UTF-8")

root_logger = logging.getLogger()
root_logger.addHandler(log_file_handler)
root_logger.removeHandler(log_file_handler)
root_logger.error("error")

del log_file_handler
```

It looks like the weakref might get lost for some reason.

See pytest-dev@92ffe42b45 / pytest-dev#4981
for more information.

1: https://github.com/python/cpython/blob/c1419578a18d787393c7ccee149e7c1fff17a99e/Lib/logging/__init__.py#L2107-L2139
blueyed added a commit to blueyed/pytest that referenced this pull request Mar 26, 2019
While it should be closed in logging's shutdown [1], the following would
still issue a ResourceWarning:

```
import logging

log_file_handler = logging.FileHandler("temp.log", mode="w", encoding="UTF-8")

root_logger = logging.getLogger()
root_logger.addHandler(log_file_handler)
root_logger.removeHandler(log_file_handler)
root_logger.error("error")

del log_file_handler
```

It looks like the weakref might get lost for some reason.

See pytest-dev@92ffe42b45 / pytest-dev#4981
for more information.

1: https://github.com/python/cpython/blob/c1419578a18d787393c7ccee149e7c1fff17a99e/Lib/logging/__init__.py#L2107-L2139
@pytest-dev pytest-dev deleted a comment from codecov bot Mar 15, 2020
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.

1 participant