-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Port TestFileTaskLogHandler tests to new Structlog logs #50990
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
Port TestFileTaskLogHandler tests to new Structlog logs #50990
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
|
Tests are failing: |
Hey @kaxil, this commit fixes the failing tests. Thanks for the feedback. |
3c0f861 to
252bfb8
Compare
|
Thanks, I have rebased your PR on main, let's see what the CI says |
o-nikolas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice fix, thanks for circling back and ensuring these tests pass :)
…l decorators and update assertions for StructuredLogMessage objects (fixes apache#50977)
…ests Use logging.getLogger(TASK_LOGGER) directly in task callables instead of ti.log to avoid AttributeError when RuntimeTaskInstance is created during test execution.
252bfb8 to
967d3ed
Compare
| def task_callable(ti): | ||
| ti.log.info("test") | ||
| def task_callable(): | ||
| logger = logging.getLogger(TASK_LOGGER) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this use structlog.get_logger instead?
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. |
Fixes #50977
Problem
Two
TestFileTaskLogHandlertests were marked with@pytest.mark.xfailbecause they were written for the old logging format but needed to work with the new structlog-based logging system:test_file_task_handler_when_ti_value_is_invalidtest_file_task_handlerThese tests were failing because they expected log data as strings but were receiving
StructuredLogMessageobjects from the new structlog implementation.Solution
Removed
@pytest.mark.xfaildecorators - (tests now pass instead of being marked as expected failures)Updated
test_file_task_handlerassertion logic:assert re.search(target_re, log_content)(regex matching on string content)assert any("test" in event for event in events(logs))(substring matching on event messages)No changes needed for
test_file_task_handler_when_ti_value_is_invalid- This test actually already used direct object access (logs[0].event) which works correctly withStructuredLogMessageobjects