Skip to content

Add logging to TAP file #94

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

Merged
merged 16 commits into from
Jan 30, 2025
Merged

Add logging to TAP file #94

merged 16 commits into from
Jan 30, 2025

Conversation

codambro
Copy link
Contributor

@codambro codambro commented Jan 24, 2025

resolves #91

This is based off the existing pytest JUnit logging support.

--tap-logging 'all'

1..6
ok 1 test_logging.py::test_ok
not ok 2 test_logging.py::test_not_ok
# def test_not_ok():
#         LOGGER.error("Running test_not_ok")
# >       assert False
# E       assert False
# 
# test_logging.py:12: AssertionError
# --- Captured Log ---
# ERROR    test_logging:test_logging.py:11 Running test_not_ok
# --- Captured Out ---
# 
# --- Captured Err ---
#
ok 3 test_logging.py::test_params[foo]
ok 4 test_logging.py::test_params[bar]

NOTE: Have PR to add logging to passing tests, but that requires an update to tappy.
tappy PR - python-tap/tappy#148
pytest-tap PR - codambro#1

To accept your contribution, please complete the checklist below.

  • Is your name/identity in the AUTHORS file alphabetically?
  • Did you write a test to verify your code change?
  • Is CI passing?

@codambro
Copy link
Contributor Author

@mblayman Could I get a review on this? I can't seem to request reviewers on this repo

Copy link
Member

@mblayman mblayman left a comment

Choose a reason for hiding this comment

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

This looks pretty decent to me. Config options are forever, practically, so I'd like to avoid adding one of these.

@codambro codambro requested a review from mblayman January 29, 2025 14:59
@codambro
Copy link
Contributor Author

codambro commented Jan 29, 2025

Believe it is ready to re-review. Some concern as the default behavior in pytest is for --show-capture=all. Meaning anybody using pytest-tap will now by default have log/out/err all added to failing test diagnostics. I don't know if there is a case where a user may want the TAP result file to be kept minimal, whereas they have a separate log file with all the test output. By combining it with --show-capture, they no longer can separate them.

I'm also considering to make these ini config options (via addini) as opposed to command line args. Since that's how JUnit result files are configured.

@mblayman
Copy link
Member

Some concern as the default behavior in pytest is for --show-capture=all.

I'm not concerned. The library makes no promises about what diagnostics it outputs. This configuration lines up with the default pytest behavior, which feels good to me.

Copy link
Member

@mblayman mblayman left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! 👍

@mblayman mblayman merged commit e8f9c78 into python-tap:main Jan 30, 2025
1 check passed
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.

Allow additional output
2 participants