Add a "collection of results" class for collection-wide actions - #5027
Add a "collection of results" class for collection-wide actions#5027happz wants to merge 2 commits into
Conversation
Sevral methods are tied to the `Result` class, but apply to a `list[Result]`. Instead, let's have a class representing a collection of results, and let that one own the collection-level operations like `summary`.
happz
added
code | no functional change
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Results collection class inheriting from list to manage test results, refactoring various steps and runs to use it instead of standard lists. Feedback suggests overriding __getitem__, __add__, and copy in Results to prevent type degradation back to a standard list during operations, using .copy() directly instead of slicing, and simplifying redundant fully qualified type annotations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| class Results(list[ResultT]): | ||
| """ | ||
| A collection of results. | ||
|
|
||
| Effectively a fancy list of results, with a few helper methods for | ||
| the collection as a whole. | ||
| """ | ||
|
|
||
| def total(self) -> dict[ResultOutcome, int]: |
There was a problem hiding this comment.
Override __getitem__, __add__, and copy in the Results class to preserve the custom Results collection type during slicing, concatenation, and copying operations. Slicing or copying a standard list subclass otherwise degrades the type back to a standard list, which can cause runtime errors if collection-specific methods (like .summary()) are called on the resulting object.
| class Results(list[ResultT]): | |
| """ | |
| A collection of results. | |
| Effectively a fancy list of results, with a few helper methods for | |
| the collection as a whole. | |
| """ | |
| def total(self) -> dict[ResultOutcome, int]: | |
| class Results(list[ResultT]): | |
| """ | |
| A collection of results. | |
| Effectively a fancy list of results, with a few helper methods for | |
| the collection as a whole. | |
| """ | |
| def __getitem__(self, item: Any) -> Any: | |
| if isinstance(item, slice): | |
| return Results(super().__getitem__(item)) | |
| return super().__getitem__(item) | |
| def __add__(self, other: list[Any]) -> 'Results[ResultT]': | |
| return Results(super().__add__(other)) | |
| def copy(self) -> 'Results[ResultT]': | |
| return Results(super().copy()) | |
| def total(self) -> dict[ResultOutcome, int]: |
| assert self.parent is not None # narrow type | ||
| assert isinstance(self.parent, Execute) # narrow type | ||
| self.parent._old_results = self.parent._results[:] | ||
| self.parent._old_results = Results(self.parent._results[:]) |
There was a problem hiding this comment.
Use the .copy() method directly on the Results collection instead of slicing with [:] and wrapping it back in Results(...).
| self.parent._old_results = Results(self.parent._results[:]) | |
| self.parent._old_results = self.parent._results.copy() |
| result_class: type[ResultT], | ||
| allow_missing: bool = False, | ||
| ) -> list[ResultT]: | ||
| ) -> tmt.result.Results[ResultT]: |
There was a problem hiding this comment.
Use the directly imported Results class instead of the redundant tmt.result.Results prefix.
| ) -> tmt.result.Results[ResultT]: | |
| ) -> Results[ResultT]: |
| self._assert_required_tests_executed() | ||
|
|
||
| def results(self) -> list["tmt.result.Result"]: | ||
| def results(self) -> 'tmt.result.Results[tmt.result.Result]': |
There was a problem hiding this comment.
Use 'Results[Result]' instead of 'tmt.result.Results[tmt.result.Result]' since both classes are already imported directly in this file.
| def results(self) -> 'tmt.result.Results[tmt.result.Result]': | |
| def results(self) -> 'Results[Result]': |
1 participant
Sevral methods are tied to the
Resultclass, but apply to alist[Result]. Instead, let's have a class representing a collection of results, and let that one own the collection-level operations likesummary.Pull Request Checklist