Skip to content

chore(generator): enable system tests#17730

Draft
ohmayr wants to merge 1 commit into
mainfrom
issue-16190-gapic-generator-system-tests
Draft

chore(generator): enable system tests#17730
ohmayr wants to merge 1 commit into
mainfrom
issue-16190-gapic-generator-system-tests

Conversation

@ohmayr

@ohmayr ohmayr commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

No description provided.

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

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.

Code Review

This pull request implements the system test session in noxfile.py to run the system test suite against Showcase, controlled by the RUN_SYSTEM_TESTS environment variable. The feedback suggests improving the parsing of this environment variable by failing fast and raising a ValueError if an invalid or empty value is provided, rather than silently falling back to a default.

Comment on lines +824 to +825
if os.environ.get("RUN_SYSTEM_TESTS", "true") == "false":
session.skip("RUN_SYSTEM_TESTS is set to false, skipping")

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.

medium

When parsing the RUN_SYSTEM_TESTS environment variable, if a user explicitly sets an invalid value (such as whitespace-only or an invalid boolean string), we should fail fast and raise an error to notify them of the invalid configuration rather than silently falling back to a default value.

    run_env = os.environ.get("RUN_SYSTEM_TESTS", "true")
    if not run_env.strip():
        raise ValueError("RUN_SYSTEM_TESTS cannot be empty or whitespace-only")
    run_system_tests = run_env.strip().lower()
    if run_system_tests not in ["true", "false"]:
        raise ValueError(f"Invalid value for RUN_SYSTEM_TESTS: {run_env}")
    if run_system_tests == "false":
        session.skip("RUN_SYSTEM_TESTS is set to false, skipping")
References
  1. When parsing environment variables, if a user explicitly sets an invalid value (such as whitespace-only), fail fast and raise an error to notify them of the invalid configuration rather than silently falling back to a default value.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant