Skip to content

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.

3 participants