-
Notifications
You must be signed in to change notification settings - Fork 167
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
Rework Step information to use logging infrastructure #1106 #1155
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Rename and simplify __str__ to get_title(). This frees up the __str__ formatting for Steps. Signed-off-by: Rouven Czerwinski <[email protected]>
Signed-off-by: Rouven Czerwinski <[email protected]>
Signed-off-by: Rouven Czerwinski <[email protected]>
Implement StepLogger which takes stepevents and turns them into logging events. Also add a Formatter which can detect steps being added as an extra to a log message and formats the step by indentation. Signed-off-by: Rouven Czerwinski <[email protected]>
Signed-off-by: Rouven Czerwinski <[email protected]>
Replace the terminalrewriting with logging. Signed-off-by: Rouven Czerwinski <[email protected]>
Used in the logging output since it implements the ConsoleProtocol. Signed-off-by: Rouven Czerwinski <[email protected]>
Used in the logging output since it implements the ConsoleProtocol. Signed-off-by: Rouven Czerwinski <[email protected]>
Signed-off-by: Rouven Czerwinski <[email protected]>
Signed-off-by: Rouven Czerwinski <[email protected]>
Logging to stderr causes a lot of unnecessary output when system capture is disabled, and isn't really necessary with the improved logging capabilities, so remove it. Users can still manually enable it with this in their conftest.py: def pytest_configure(config): logging.basicConfig( level=logging.INFO, format='%(levelname)7s: %(message)s', stream=sys.stderr, ) Signed-off-by: Joshua Watt <[email protected]>
Emantor
force-pushed
the
topic/logging_work
branch
2 times, most recently
from
April 27, 2023 13:25
4c4caa1
to
9621e39
Compare
Bastian-Krause
requested changes
Apr 28, 2023
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.
The changes in the Step class are weird:
- first,
Step.__str__
is dropped, Step.get_title is added - then,
Step.__str__
is readded - finally,
Step.get_title
andStep.__str__
are dropped again
labgrid/pytestplugin/reporter.py
is modified, then removed completely. I guess these commits can be squashed?
We should add a note that --lg-colored-steps
has currently no effect and what's the plan for the future.
Reworks the way that step and console logging is done with the intention of respecting the users pytest logging configuration, namely the --log-(cli-|file-)(format|level) command line and corresponding pytest.ini options. This works by having the StepFormatter class take a "parent" argument which is pytests original formatter, and having it pass logs to it (after some munging) instead of doing formatting itself. For this to work as intended, and also so step and console logging works *without* the StepFormatter class, the formatting of log messages is now done when the messages are logged, instead of in the StepFormatted. This makes the "$(message)s" specifier in the formater handler work as expected. The StepFormatter adds (optional) indentation behavior on top of this to make logs prettier. Indentation is configured by passing extra properties when constructing the log messages. These properties are handled by the StepFromatter and indicate how much the current message should be indented, and how much any subsequent messages should be indented if they do not otherwise specify an indentation level. Signed-off-by: Joshua Watt <[email protected]>
Applies the StepFormatter to both the report (log messages shown on test failure) and the caplog (log messages shown when using the caplog fixture) Signed-off-by: Joshua Watt <[email protected]>
Adds support for logging writes to the serial console Signed-off-by: Joshua Watt <[email protected]>
The users have been removed in previous commits, remove the functions themself. Signed-off-by: Rouven Czerwinski <[email protected]>
The logger needs to use step.get_title() instead of str(step) to match the new step API Signed-off-by: Joshua Watt <[email protected]>
Fixes the step reporter tests to match the new logging format Signed-off-by: Joshua Watt <[email protected]>
Stop the pytest logger when the main loop exits. This is primarily to fix the documentation examples, which were failing to build because the StepLogger singleton wasn't stopped between test code fragments Signed-off-by: Joshua Watt <[email protected]>
Emantor
force-pushed
the
topic/logging_work
branch
from
April 28, 2023 10:35
ba93928
to
afc9d4d
Compare
Adds release documentation for the logging changes Signed-off-by: Joshua Watt <[email protected]> [[email protected]: rebased on master] Signed-off-by: Rouven Czerwinski <[email protected]>
Removes the StepReporter and ColoredStepReporter, as the former is now provided by labgrid.stepreporter and the later isn't implemented yet. Signed-off-by: Joshua Watt <[email protected]>
Add the source information in the form of absolute path, filename and line number to the step. This can then be used by the logging infrastructure to fix the information in the logging record. Signed-off-by: Rouven Czerwinski <[email protected]>
Fix the record in the formatter to not emit filenames and line number for the StepLogger and instead use the information retrieved in the step decorator. This provides much more useful line number information in default logging formatters. Signed-off-by: Rouven Czerwinski <[email protected]>
The StepReporter should be replaced by the StepLogger and basicConfig from labgrid.logging. It uses the python logging infrastructure and a better output formatting. Signed-off-by: Rouven Czerwinski <[email protected]>
Also allow setting the indentation and add the formatter created by basicConfig as the parent of the StepFormatter. This ensures that a user supplied format is repected when passed into basicConfig. Signed-off-by: Rouven Czerwinski <[email protected]>
We set the CONSOLE level in between INFO and DEBUG. This affords us some space for future levels. Also assert if the DEBUG level or INFO level where changed in a way that would mean CONSOLE would be below debug. Signed-off-by: Rouven Czerwinski <[email protected]>
This option is unused. Remove it. Signed-off-by: Rouven Czerwinski <[email protected]>
Change the name of the created SerialLoggers to SerialLogger.<target-name>.<source-name>. Signed-off-by: Rouven Czerwinski <[email protected]>
Instead of having a true/false long_result keyword argument, use it to setup the length limit itself which defaults to 100 if unset. Signed-off-by: Rouven Czerwinski <[email protected]>
Looks a bit cleaner to my eyes and also employs repr() to ensure we can handle newlines. Signed-off-by: Rouven Czerwinski <[email protected]>
Instead of → Step run cmd="sleep" fix the formatting: → BareboxDriver.run(cmd="sleep") Where we no longer use event.step.__class__.__name__ (which is the same for all events) and use the name of step.source instead (which was always the intention). Also switch to a more function style formatting. Signed-off-by: Rouven Czerwinski <[email protected]>
Signed-off-by: Rouven Czerwinski <[email protected]>
Instead of overwriting the format, don't set it to use the default format from the labgrid logging module. Signed-off-by: Rouven Czerwinski <[email protected]>
Use the new CONSOLE log level if -vv verbosity is used and a state transition via a configuration file is requested. Signed-off-by: Rouven Czerwinski <[email protected]>
Replace the default log format with our custom log format defined in the logging module. See the comment for the preferred approach which is sadly not possible for now. Signed-off-by: Rouven Czerwinski <[email protected]>
Rework the StepReporter tests to use the StepLogger instead. Signed-off-by: Rouven Czerwinski <[email protected]>
We got more and more users using the vt100 removal regex, add it to the helper to use it from a central location. Signed-off-by: Rouven Czerwinski <[email protected]>
Use the centrally compiled helper and remove the class property indirection. Signed-off-by: Rouven Czerwinski <[email protected]>
Use the central helper. Signed-off-by: Rouven Czerwinski <[email protected]>
Import and use the central helper. Signed-off-by: Rouven Czerwinski <[email protected]>
Use the central helper. Signed-off-by: Rouven Czerwinski <[email protected]>
Use the central re_vt100 helper. Signed-off-by: Rouven Czerwinski <[email protected]>
Emantor
force-pushed
the
topic/logging_work
branch
from
April 28, 2023 12:22
5fd28d4
to
a2de90f
Compare
Bastian-Krause
approved these changes
Apr 28, 2023
jluebbe
approved these changes
Apr 28, 2023
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Hopefully the final iteration of the logging rework.
Checklist
Closes #1106