Skip to content

Make FmfContext read-only#4892

Open
LecrisUT wants to merge 2 commits into
teemtee:mainfrom
LecrisUT:feat/read-only-fmfcontext
Open

Make FmfContext read-only#4892
LecrisUT wants to merge 2 commits into
teemtee:mainfrom
LecrisUT:feat/read-only-fmfcontext

Conversation

@LecrisUT
Copy link
Copy Markdown
Member

@LecrisUT LecrisUT commented May 15, 2026

First step towards unifying this with fmf.context.Context. Will make an fmf PR equivalent to have a similar Mapping interface (Mapping[str, set[ContextValue]]) and then try to unify the types and constructors.

fmf counterpart teemtee/fmf#300

LecrisUT added 2 commits May 16, 2026 00:03
Signed-off-by: Cristian Le <git@lecris.dev>
Signed-off-by: Cristian Le <git@lecris.dev>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors FmfContext to inherit from collections.abc.Mapping instead of dict, updating its instantiation across the codebase to use keyword arguments. An issue was identified where the format utility still relies on isinstance(value, dict), which will cause FmfContext objects to be formatted incorrectly. Update _VALUE_FORMATTERS and _format_dict to handle Mapping types.

Comment thread tmt/utils/__init__.py
def format(
key: str,
value: Union[None, float, bool, str, list[Any], dict[Any, Any]] = None,
value: Union[None, float, bool, str, list[Any], Mapping[Any, Any]] = None,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

While the type hint is correctly updated to Mapping, the underlying implementation in _format_value still checks for isinstance(value, dict). Since FmfContext is now a Mapping but not a dict, it will not be formatted as a dictionary, but will fall back to its default string representation (<tmt.utils.FmfContext object at ...>). This will likely cause the test_format_value test for fmf context to fail.

To fix this, update _VALUE_FORMATTERS to check for collections.abc.Mapping instead of dict, and update the signature of _format_dict to accept a Mapping.

@LecrisUT LecrisUT added the ci | full test Pull request is ready for the full test execution label May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci | full test Pull request is ready for the full test execution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant