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

Access captures logs in teardown #3117

Merged
merged 2 commits into from
Jan 20, 2018
Merged

Access captures logs in teardown #3117

merged 2 commits into from
Jan 20, 2018

Conversation

boxed
Copy link
Contributor

@boxed boxed commented Jan 15, 2018

This is a fix for the problem seen here: eisensheng/pytest-catchlog#37

I would like to validate some logs globally for the entire test suite. The problem is that caplog gives you one bucket of logs for the phase you're currently in, but I want to check the 'call' bucket when I'm in 'teardown'. This change makes it possible to access all the buckets from all the other.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 92.561% when pulling a1a790e on boxed:access_logs_in_teardown into 3181718 on pytest-dev:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 92.562% when pulling a1a790e on boxed:access_logs_in_teardown into 3181718 on pytest-dev:master.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Looks good as a first iteration, thanks!

Some things missing:

  • A test for the new feature
  • Please rebase this on the features branch because it is a new functionality
  • A new change log entry: a file changelog/3117.feature with a brief description of the change, from the POV of an user.

Also I think we should mention this in the documentation, in logging.rst.

Thanks again @boxed!

@boxed
Copy link
Contributor Author

boxed commented Jan 18, 2018

I can do all that, but notice that I'll document caplog._item, which looks like an internal thing, but now will become a part of the documented API.

@boxed boxed changed the base branch from master to features January 18, 2018 11:10
@boxed
Copy link
Contributor Author

boxed commented Jan 18, 2018

@nicoddemus Changes made.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 92.628% when pulling b67badc on boxed:access_logs_in_teardown into 1fd67c9 on pytest-dev:features.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.004%) to 92.626% when pulling b67badc on boxed:access_logs_in_teardown into 1fd67c9 on pytest-dev:features.

@nicoddemus
Copy link
Member

I can do all that, but notice that I'll document caplog._item, which looks like an internal thing, but now will become a part of the documented API.

@boxed good point! I propose we create a get_handler(when) method in the caplog fixture:

def get_handler(self, when):
    """..."""
    return self._item.catch_log_handlers.get(when)

This way we don't expose the entire Item object.



@pytest.fixture
def logging_during_setup_and_teardown(caplog):
Copy link
Member

Choose a reason for hiding this comment

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

The test looks good, thanks!

@boxed
Copy link
Contributor Author

boxed commented Jan 19, 2018

I've made the suggested change to introduce a get_handler method. I also added myself to AUTHORS.

@boxed
Copy link
Contributor Author

boxed commented Jan 19, 2018

I've added a little segment in the docs too... it's not the prettiest text, so suggestions are welcome!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 92.627% when pulling b9fb1cb on boxed:access_logs_in_teardown into 1fd67c9 on pytest-dev:features.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 92.627% when pulling 7ea5a22 on boxed:access_logs_in_teardown into 1fd67c9 on pytest-dev:features.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks @boxed!

@nicoddemus nicoddemus merged commit 3b3d237 into pytest-dev:features Jan 20, 2018
@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 92.627% when pulling c4c968f on boxed:access_logs_in_teardown into 1fd67c9 on pytest-dev:features.

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.

None yet

3 participants