Skip to content
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
merged 43 commits into from
Apr 28, 2023

Conversation

Emantor
Copy link
Member

@Emantor Emantor commented Apr 27, 2023

Description
Hopefully the final iteration of the logging rework.

Checklist

  • PR has been tested

Closes #1106

@Emantor Emantor added this to the v23.1 milestone Apr 27, 2023
@Emantor Emantor self-assigned this Apr 27, 2023
Emantor and others added 11 commits April 27, 2023 15:10
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]>
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]>
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 Emantor force-pushed the topic/logging_work branch 2 times, most recently from 4c4caa1 to 9621e39 Compare April 27, 2023 13:25
@Emantor Emantor assigned jluebbe and Bastian-Krause and unassigned Emantor Apr 28, 2023
Copy link
Member

@Bastian-Krause Bastian-Krause left a 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 and Step.__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.

labgrid/logging.py Outdated Show resolved Hide resolved
labgrid/logging.py Outdated Show resolved Hide resolved
labgrid/logging.py Outdated Show resolved Hide resolved
labgrid/logging.py Outdated Show resolved Hide resolved
labgrid/logging.py Outdated Show resolved Hide resolved
labgrid/logging.py Outdated Show resolved Hide resolved
labgrid/pytestplugin/hooks.py Outdated Show resolved Hide resolved
labgrid/pytestplugin/hooks.py Outdated Show resolved Hide resolved
tests/test_steplogger.py Outdated Show resolved Hide resolved
tests/test_steplogger.py Outdated Show resolved Hide resolved
JoshuaWatt and others added 7 commits April 28, 2023 12:34
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]>
JoshuaWatt and others added 23 commits April 28, 2023 14:22
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 Emantor merged commit f71fcf2 into labgrid-project:master Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants