Skip to content
This repository was archived by the owner on Apr 18, 2018. It is now read-only.

Merging into pytest-catchlog? #1

Closed
The-Compiler opened this issue Nov 9, 2015 · 19 comments
Closed

Merging into pytest-catchlog? #1

The-Compiler opened this issue Nov 9, 2015 · 19 comments

Comments

@The-Compiler
Copy link

Hey,

there's unfortunately already some confusion around pytest logging plugins, as there's the abandoned pytest-capturelog and the maintained pytest-catchlog fork. Unfortunately, having another pytest-logging plugin only increases that confusion 😉

What are your thoughts about merging the functionality of pytest-logging into pytest-catchlog? It already has --log-format and --log-date-format arguments, so I guess only the -v handling is what's missing from it?

/cc @hpk42 @eisensheng @abusalimov

@s0undt3ch
Copy link
Contributor

Let me say upfront that I have no problems merging the projects, however, we have at least two issues:

  • pytest-catchlog is meant to capture logging while pytest-logging is meant to display logging; Considering only the names of the plugins, pytest-logging is broad enough to include capturing logs, while catchlog is not broad enough to make users think that is also takes care of displaying the log messages(not really a big problem and I'm probably biased towards my plugin name 😄)
  • Licensing.pytest-logging was created using the human resources of SaltStack(me and my time), and while SaltStack has no issues with open source projects(it actually contributes back to a lot of them), it releases them using the Apache 2.0 license.

Would you be interested in merging both plugins but under pytest-logging instead(since it's a broader name)? We would actually merge repositories so that no git history is lost and all of the catchlog contributors hard work is given due credit; we could even move the repository to pytest-dev
Would you mind dual licensing the plugin in order to keep my contribution to the plugin under Apache 2.0?

If the answer to both the above questions is no, I'm sure we can still work something out but I'll need to bring this matter to SaltStack to get permission to release my work under the MIT license.

@The-Compiler
Copy link
Author

There actually was some discussion on the pytest-dev mailinglist to rename pytest-catchlog to pytest-logging, it's just that you were quicker 😉 See e.g. https://mail.python.org/pipermail/pytest-dev/2015-September/003166.html

We're discussing about moving pytest-catchlog to pytest-dev here: eisensheng/pytest-catchlog#4

I've not tried pytest-logging yet - apart from the commandline options, do I understand it properly that it displays the log when a test failed? If so, that's essentially what pytest-catchlog does as well 😉

In the end, those are questions which @eisensheng and @abusalimov will need to answer. I guess the rename will cause some confusion as well, but I guess less than having three logging pluggins 😉

I'm fine with both pytest-catchlog and pytest-logging as names. I loved @eisensheng's idea with the dream catcher as a logo, but I agree pytest-logging is a nice canonical name as well.

Note pytest-catchlog is based on pytest-capturelog which is licensed under the MIT license. If I interpret it correctly (IANAL), it should be possible to dual-license it without the explicit agreement of the original author (Meme Dough, who unfortunately isn't reachable anymore).

@s0undt3ch
Copy link
Contributor

Well, pytest-logging displays logging messages to the console, period. Ie, there's no logic involved on showing or not the log messages if the test fails or not. The only logic involved is setting the log level on a sys.stderr stream handler.

The first time I wrote this logging code was on a project which was using py.test to run it's tests and we needed some background logs to see what was going on(not necessarily when a test failed). I then needed to work on a pytest plugin which needs to do some boilerplate work before actually running any tests and I ended up adding the same code to it.
Of course, I soon concluded that I should not be rewriting or copy/pasting the same code whenever I needed that kind of logging information and that's why I released pytest-logging.

pytest-catchlog makes total sense to exist and is on a totally different league(which makes mine rather simplistic), I was even planning on using it to assert some log messages were logged for yet another project of ours.

I'll wait for the replies of all the @ "tagged" users...

@abusalimov
Copy link

Well, pytest-logging displays logging messages to the console, period.

Does it? Just tried it on one of my projects: it does not display logs unless a test case fails. And when it fails, the log output is just mixed into stderr.

Comparing to pytest-catchlog, what additional features does this plugin provide? I mean what exactly could I achieve with pytest-logging, but not using pytest-catchlog. I can see -v so far, and a broader name, of course. 😉

@s0undt3ch
Copy link
Contributor

Oh, yeah, I forgot to mention, you need to disable pytest output capturing, -s, then you'll see all your logging happen in real-time, and yes, it will be mixed into stderr.

@s0undt3ch
Copy link
Contributor

Unless there's a better way of showing the logs without disabling pytest output capturing? I'm pretty new to pytest, and let's not talk about pytest internals....

@abusalimov
Copy link

Ah, I see... Didn't know about the -s flag before, thanks! 👍

In pytest-catchlog we use slightly different approach, and add a separate "captured log" output section, which involves capturing, obviously. I'm not sure whether is possible to make it work nice with -s and --capturing=..., though I should be, I guess.

@s0undt3ch
Copy link
Contributor

I imagine there should not be any troubles since you use your own handler to take care of capturing...

@The-Compiler
Copy link
Author

That sounds pretty much like the default behaviour of logging/pytest - logging logs to stderr, and pytest displays that when you run it with -s.

@s0undt3ch - if we don't get to a conclusion about merging, would you consider renaming pytest-logging? If a new user searches for logging plugins for pytest, I'd guess it'd be better for them to find pytest-catchlog rather than pytest-logging - but logging is the more generic name, so probably what a new user would pick. It still seems to me like everything this plugin does is covered by pytest-catchlog too, except of the -v thing - and then of course a lot more 😉.

I hope this doesn't sound offending - sorry if it does, that's not my intention. 😄

@s0undt3ch
Copy link
Contributor

And -v in pytest-logging only tweaks the level of the "CONSOLEHANDLER"...

@s0undt3ch
Copy link
Contributor

Honestly, I also prefer merging the plugins and I just asked for guidance from SaltStack regarding this subject.

Regarding your line, "That sounds pretty much like the default behaviour of logging/pytest - logging logs to stderr, and pytest displays that when you run it with -s.", are you saying that pytest already does this? Because it didn't the first time I wrote this code... Or are you saying that its the default behavior of logging and pytest on it's own?

@The-Compiler
Copy link
Author

Try running this without a plugin and with -s:

import logging
import time

def test_foo():
    logging.error("foo")
    time.sleep(1)
    logging.error("bar")

If you want another loglevel, you'd need to set the loglevel, e.g. using a autouse fixture:

import logging
import time
import pytest

@pytest.fixture(autouse=True)
def set_loglevel():
    logging.basicConfig(level=logging.DEBUG)


def test_foo():
    logging.debug("foo")
    time.sleep(1)
    logging.debug("bar")

So unless I misunderstood something (if so, please let me know!), pytest-logging adds the log format options and the -v behaviour - but otherwise does the same as logging/pytest already do. That's why I originally proposed "merging" it into catchlog - the only thing needed would be to add the -v behaviour.

But I'd say let's wait for @eisensheng's opinion on this.

@abusalimov
Copy link

Or are you saying that its the default behavior of logging and pytest on it's own?

You're almost right. The default behavior is to redirect logs of level WARNING and above to stderr.

Basically, the following lines would be enough to output everything:

import logging
def pytest_configure():
    logging.basicConfig(level=logging.NOTSET)  # or whatever level you prefer

@s0undt3ch
Copy link
Contributor

Ok, so, it seems that my plugin only adds the log level tweak feature at the CLI level... (guess I forgot my original findings on the matter)

@s0undt3ch
Copy link
Contributor

Sorry, wrong. On top of -v my plugin sets up a stream handler, which can be achieved using the solution @abusalimov provided

@s0undt3ch
Copy link
Contributor

For now, I'm "occupying" the pytest-logging name, however, I'm leaning towards contributing what my plugin does into catchlog and then withdrawing my plugin for when catchlog decides to rename itself into pytest-logging. This is still all depending on the feedback I get from SaltStack but I imagine that it should not be a problem.

@s0undt3ch
Copy link
Contributor

Ok, I've got the 👍 from SaltStack. I'll be creating a PR against catchlog soon to implement eisensheng/pytest-catchlog#14. ... under MIT, so even less trouble.

@DavidAntliff
Copy link

Perhaps a better name for this library, given this discussion, is pytest-loglevel?

@s0undt3ch
Copy link
Contributor

There's no reason why you should use this plugin anymore.
Upgrade pytest and it'll provide you with what this plugin also provided.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants