Repository navigation
Update RDK SB subtree - #5926
Conversation
aa721db4091 Add support files required to run RDK tests using `cobalt/starboard/tools/testing/test_runner.py` ecce60e22f3 Add default args to build RDK git-subtree-dir: starboard/contrib/rdk git-subtree-split: aa721db409100b273cc60fed259db50bed91a1d3
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @andrunko, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
As gemini-code-assist, I've reviewed this pull request. The primary goal is to update the RDK subtree located at starboard/contrib/rdk to the latest version from the upstream 25.lts.youtube branch using a git subtree pull. This update specifically includes changes necessary to enable the use of test_runner.py on the RDK platform. The changes involve adding new configuration and test filter files for the RDK ARM architecture within the updated subtree.
Highlights
- Subtree Update: Updates the
starboard/contrib/rdksubtree to the latest25.lts.youtubebranch from the upstream Cobalt repository usinggit subtree pull. - RDK ARM Configuration Added: Adds a new file
gyp_configuration.pyspecifically for the RDK ARM platform, defining platform-specific build and test configurations. - RDK ARM Test Filters Added: Adds a new file
test_filters.pyfor the RDK ARM platform, providing a place to define platform-specific test filtering rules (currently inheriting from shared filters). - Test Runner Support: Includes changes required to support running tests using
test_runner.pyon the RDK platform, specifically by including Evergreen compatible tests in the RDK ARM configuration.
Changelog
- starboard/contrib/rdk/src/third_party/starboard/rdk/arm/gyp_configuration.py
- New file added.
- Defines the
RdkPlatformConfigclass for RDK ARM platform configuration. - Overrides
GetTestTargetsto includeEVERGREEN_COMPATIBLE_TESTS.
- starboard/contrib/rdk/src/third_party/starboard/rdk/arm/test_filters.py
- New file added.
- Defines the
RdkTestFiltersclass for RDK ARM test filtering. - Overrides
GetTestFiltersto return filters from the shared RDK configuration.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Subtree pulled anew,
Bringing tests for RDK,
Code grows, ever green.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request updates the RDK subtree and introduces new Python configuration files for the RDK ARM platform, specifically related to GYP configuration and test filters. The changes appear to be part of the effort to enable test_runner.py on RDK. The structure of the new files is consistent with common patterns for platform-specific configurations.
Summary of Findings
- Redundant Method Override Clarity: The
GetTestFiltersmethod instarboard/contrib/rdk/src/third_party/starboard/rdk/arm/test_filters.pyis overridden but currently performs no action beyond calling the superclass's implementation. This could be clarified with a comment regarding its intent (e.g., as a placeholder for future ARM-specific filters). - Potential Copyright Year Mismatch: The copyright year in the newly added files (
gyp_configuration.py,test_filters.py) is listed as 2025. This might be a template default or a typo and could be worth verifying. (Not commented inline due to review settings for low severity issues).
Merge Readiness
The pull request sets up necessary configurations for the RDK ARM platform. However, there is one medium-severity suggestion regarding the clarity of a method override in test_filters.py. Addressing this would improve the maintainability of the code.
I recommend that these changes be considered before merging. As an AI, I am not authorized to approve pull requests; please ensure further human review and approval.
| def GetTestFilters(self): | ||
| filters = super().GetTestFilters() | ||
| return filters |
There was a problem hiding this comment.
The GetTestFilters method in RdkTestFilters currently overrides the parent method only to call super().GetTestFilters() and return its result without any modification. This makes the override functionally a no-op.
Is this intended as a placeholder for future ARM-specific filters, or are no specific filters anticipated for this platform configuration at this moment?
Adding a comment to clarify the intent would improve code clarity and maintainability. For example, if it's a placeholder for potential future additions, a TODO or explanatory comment would be helpful.
| def GetTestFilters(self): | |
| filters = super().GetTestFilters() | |
| return filters | |
| def GetTestFilters(self): | |
| # TODO: Add any ARM-specific test filters here if different from shared configuration. | |
| filters = super().GetTestFilters() | |
| return filters |
Update RDK subtree under
starboard/contrib/rdkto latest from upstream's25.lts.youtubebranch -- this includes changes required to use test_runner.py on RDK.Command used:
b/415848657