Skip to content

Conversation

@antonio-amjr
Copy link
Contributor

Fix: project-chip/certification-tool#825
Depends on: project-chip/certification-tool-backend#289

Description

This PR implements the double dash (--) separator syntax in the th-cli run-tests command, enabling users to pass temporary test parameters that apply only to the current test execution without persisting to project configuration.

💡 Solution

Implemented standard CLI convention of using -- separator to pass extra arguments that:

  • Apply only to the current test execution
  • Don't persist to project configuration
  • Support SDK test parameter formats (endpoint: 2, int-arg: some-arg:1234, etc.)
  • Work independently or combined with config files

Usage Examples

# Extra args only (temporary)
th-cli run-tests -t TC-ACE-1.1 -- --endpoint 2

# Config file + extra args (file persists, extra args don't)
th-cli run-tests -t TC-ACE-1.1 --config base.json -- --int-arg some-arg:3

# Multiple extra args
th-cli run-tests -t TC-ACE-1.1 -- --int-arg some-arg:3 --bool-arg flag:true --string-arg text-arg:beef

Testing

Manual Testing

# Test 1: Extra args don't persist
th-cli run-tests -t TC-ACE-1.1 -- --endpoint 2
# Verify: Check that project config doesn't contain endpoint 2

# Test 2: Config file does persist
th-cli run-tests -t TC-ACE-1.1 --config test.json
# Verify: project config updated with test.json contents

# Test 3: Combined behavior
th-cli run-tests -t TC-ACE-1.1 --config base.json -- --int-arg some-arg:3
# Verify: project config has base.json, execution only uses base.json + int-arg 3

Automated Testing

  • Sanity tests pass ✅
  • Unit Tests Ok ✅

To Reviewers: Please pay special attention to:

  1. Unit Tests created
  2. The _parse_extra_args() function logic
  3. The temporary execution config feature
  4. Documentation clarity

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @antonio-amjr, 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!

This pull request introduces a significant enhancement to the th-cli run-tests command by enabling the dynamic passing of temporary test parameters to SDK tests. This feature allows users to override or add specific test configurations for a single execution without altering the project's saved settings, greatly improving the flexibility and isolation of test runs. The changes include updates to the CLI argument parsing, API models to accommodate execution-specific configurations, and comprehensive documentation and testing to support the new functionality.

Highlights

  • New Feature: Double-Dash Extra Arguments: The th-cli run-tests command now supports a double-dash (--) separator, allowing users to pass additional arguments directly to the SDK container Python tests.
  • Temporary Test Parameters: Arguments passed after the -- separator are temporary and apply only to the current test execution. They do not persist to the project's configuration, ensuring clean and isolated test runs.
  • SDK Test Parameter Support: The new functionality supports various SDK test parameter formats, such as endpoint: 2, int-arg: some-arg:1234, bool-arg: flag:true, and string-arg: text-arg:beef.
  • Flexible Usage: These extra arguments can be used independently or combined with existing configuration files, providing greater flexibility for test execution.
  • New _parse_extra_args() Function: A new utility function, _parse_extra_args(), has been implemented to correctly parse the arguments provided after the double-dash separator into a dictionary format suitable for test parameters.
  • Configuration Isolation: The implementation ensures that the test_run_config used for execution is a deep copy of the project configuration, preventing temporary extra arguments from inadvertently modifying persistent settings.
  • Documentation Updates: The README.md has been updated to clearly document the new -- <extra-sdk-args> option, including usage examples and a list of common SDK test arguments.
  • Comprehensive Testing: New unit tests for _parse_extra_args() and integration tests for the run_tests command with extra arguments have been added to ensure the feature works as expected under various scenarios, including basic usage, multiple arguments, and combination with config files.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

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 by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

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 pull request 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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

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.

Footnotes

  1. 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.

Copy link
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 introduces a valuable feature allowing users to pass temporary arguments to th-cli run-tests using the -- separator. The implementation is robust, correctly using click context settings and ensuring configuration isolation with copy.deepcopy. The new _parse_extra_args function effectively handles argument parsing, and the accompanying unit tests are comprehensive, covering various scenarios and edge cases. The documentation updates in README.md are clear and provide helpful examples for the new functionality. Overall, this is a well-executed feature enhancement.

- `--string-arg <name>:<value>` - Pass string argument to test
- `--hex-arg <name>:<hex-value>` - Pass hex value argument to test

**Note:** These extra arguments are applied to ALL Python tests in the test run. Invalid arguments will cause test failures, so ensure the arguments are valid for the SDK test framework.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you confirm that this only work for ALL Python tests? I was assuming all extra parameters will be used as test_parameter in project config and in this case it should work for YAML scripts as well

params: dict[str, str] = {}
i = 0

while i < len(args):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you check if this could work with argparse lib?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants