chore: dry tests

[b-long]

No description provided.

[gemini-code-assist]

Summary of Changes

Hello @b-long, I'm Gemini Code Assist¹! 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 focuses on improving the maintainability and readability of the integration test suite. By abstracting the common patterns for building otdfctl commands into reusable helper functions, it eliminates redundant code and streamlines future test development and modifications.

Highlights

• New Helper Functions for otdfctl Commands: Introduced get_otdfctl_base_command, build_otdfctl_encrypt_command, and build_otdfctl_decrypt_command in tests/support_cli_args.py to centralize the construction of otdfctl command-line arguments, reducing duplication across tests.
• Refactored Integration Tests: Updated multiple integration test files (test_cli_comparison.py, test_tdf_reader_integration.py, test_cli_integration.py, test_cli_tdf_validation.py) to utilize the newly created helper functions, significantly reducing boilerplate code for subprocess.run calls.
• Improved Test Robustness: Modified error handling in test_tdf_reader_integration.py and test_cli_integration.py from 'skip test' to 'fail fast' when otdfctl commands fail, ensuring more immediate feedback on issues.

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

[gemini-code-assist]

Code Review

This pull request does a great job of refactoring the test suite to be more DRY by introducing helper functions for building otdfctl commands. This significantly reduces code duplication across multiple test files.

I've found a few areas for improvement in the new helper functions in tests/support_cli_args.py:

• There's a potential bug in build_otdfctl_encrypt_command regarding the order of command-line arguments, which could lead to test failures.
• The type hints could be more specific to improve code clarity and maintainability.

I've left detailed comments with suggestions for these points. Once they are addressed, this will be a solid improvement to the codebase.

[b-long]

/gemini review

[gemini-code-assist]

Code Review

This pull request does a great job of refactoring the tests to be more DRY by introducing helper functions for building CLI commands, which significantly improves maintainability. I've identified a few issues in the new tests/support_cli_args.py helper module, including a critical bug related to an incorrect path, a circular dependency, and a minor issue with missing type hints. Once these are addressed, the refactoring will be solid.

[b-long]

/gemini review

[gemini-code-assist]

Code Review

This pull request effectively reduces code duplication in the integration tests by introducing helper functions to build otdfctl commands. The new functions in tests/support_cli_args.py centralize command creation, making the tests cleaner and easier to maintain. The refactoring is well-executed across all modified test files. I have one suggestion to improve the type hint for the run_cli_inspect function for better accuracy.

[b-long]

/gemini review

[gemini-code-assist]

Code Review

This pull request does a great job of refactoring the integration tests to be more DRY by introducing helper functions for building CLI commands. The changes significantly improve the readability and maintainability of the test suite. I've added a few suggestions to further improve type safety and remove some now-redundant code following the refactor. Overall, this is a solid improvement.

[b-long]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great step towards improving the test suite by refactoring duplicated code into helper functions, following the DRY principle. The changes significantly enhance the maintainability and readability of the integration tests. I've identified a couple of areas in the new helper functions where a hardcoded flag could be replaced by a more dynamic helper that already exists, which would make the tests more robust. My detailed comments are below.

[b-long]

/gemini review

[gemini-code-assist]

Code Review

This pull request does a great job of refactoring the tests to be more DRY by introducing command builder functions. This significantly improves maintainability.

I've left a few comments on how to make the new helper functions even more robust by avoiding redundant configuration lookups, which can prevent potential inconsistencies.

Additionally, I noticed that some test commands are still constructed manually (e.g., cli_decrypt_cmd in test_cli_comparison.py). It would be great to refactor these as well for consistency. There also appears to be an inconsistency in how the Python CLI is invoked in tests: some use sys.executable while others use uv run python. Standardizing on one approach would be beneficial.

[b-long]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great refactoring effort to make the test suite more DRY. You've successfully extracted duplicated logic for building CLI commands and handling subprocess errors into shared helper functions. This significantly improves the maintainability and readability of the tests. My review includes a couple of suggestions to make the new helper functions more idiomatic within the pytest framework by using pytest.fail() for test failures. This will lead to clearer and more informative test reports.

[b-long]

/gemini review

[gemini-code-assist]

Code Review

This pull request does an excellent job of refactoring the tests to be more DRY by extracting common command-building and error-handling logic into new support modules. This significantly improves the maintainability and readability of the integration tests. However, the refactoring has introduced a critical regression where target_mode functionality has been lost. I've also included a few suggestions for code cleanup.

[gemini-code-assist]

This refactoring has introduced a regression. The function _generate_target_mode_tdf still accepts a target_mode argument, but it's no longer used in the call to build_otdfctl_encrypt_command. This means the function doesn't generate TDFs for a specific target mode anymore, which contradicts its name and purpose. The FIXME in the docstring also highlights this issue. This functionality needs to be restored in the new helper function.

[gemini-code-assist]

This helper function is missing support for target_mode, which has caused a regression in _generate_target_mode_tdf in conftest.py. The target_mode parameter should be added to this function and included in the constructed command. The FIXME on line 65 should also be removed and the docstring updated accordingly.

def build_otdfctl_encrypt_command(
    platform_url: str,
    creds_file: Path,
    input_file: Path,
    output_file: Path,
    mime_type: str = "text/plain",
    attributes: list[str] | None = None,
    tdf_type: str | None = None,
    target_mode: str | None = None,
) -> list[str]:
    """Build otdfctl encrypt command.

    Args:
        platform_url: Platform URL like "http://localhost:8080"
        creds_file: Path to credentials file
        input_file: Path to the input file to encrypt
        output_file: Path where the TDF file should be created
        mime_type: Optional MIME type for the input file
        attributes: Optional list of attributes to apply
        tdf_type: TDF type (e.g., "tdf3", "nano")
        target_mode: Target TDF spec version (e.g., "v4.2.2", "v4.3.1")
    """

    cmd = get_otdfctl_base_command(platform_url, creds_file)
    cmd.append("encrypt")
    cmd.extend(["--mime-type", mime_type])

    # Add attributes if provided
    if attributes:
        for attr in attributes:
            cmd.extend(["--attr", attr])

    if tdf_type:
        cmd.extend(
            [
                "--tdf-type",
                tdf_type,
            ]
        )

    if target_mode:
        cmd.extend(["--target-mode", target_mode])

    cmd.extend(
        [
            str(input_file),
            "-o",
            str(output_file),
        ]
    )
    return cmd

[gemini-code-assist]

This commented-out function definition should be removed to avoid code clutter. If this is a placeholder for future work, it would be better to track it in a ticket or issue rather than leaving commented-out code in the codebase.

[gemini-code-assist]

This commented-out function definition should be removed to avoid code clutter. If this is a placeholder for future work, it would be better to track it in a ticket or issue rather than leaving commented-out code in the codebase.