Refactor chromium_tests/steps.py to reduce use of 'kwargs' [chromium/tools/build : main]

0 views

Skip to first unread message

Ben Pastene (Gerrit)

unread,

Jul 8, 2026, 5:52:30 PM (24 hours ago) Jul 8

to Luis Jimenez Rodriguez, Code Review Nudger, Jeffrey Yu, LUCI CQ, chromium...@chromium.org, infra-rev...@chromium.org

Attention needed from Jeffrey Yu and Luis Jimenez Rodriguez

Ben Pastene added 3 comments

File recipes/recipe_modules/chromium_tests/steps.py

Line 142, Patchset 2 (Latest):

    Args:
      filter_flag: Flag argument used to specify test filters.
      filter_delimiter: The delimiter to use between values when specifying
        test filters.
      repeat_flag: Flag argument used to define how many times to repeat tests.
      retry_limit_flag: Flag argument used to define the upper limit of
        retries.
      run_disabled_flag: Flag argument used to run disabled tests.
      batch_limit_flag: Flag argument used to set how many tests run in a
        given shard.

Ben Pastene . unresolved

All of this is already documented in the list of attrs defined at the top of the class. So I don't think this is adding much value.

Line 240, Patchset 2 (Latest): )

Ben Pastene . unresolved

Same. This is unnecessary duplication IMO

Line 835, Patchset 2 (Latest): target_name: The target name for the test. If not set, defaults to `name`.

Ben Pastene . unresolved

I would just get rid of this method's docstring all together, and add this bit into its class-level docstring.

Open in Gerrit

Related details

Attention is currently required from:

Jeffrey Yu
Luis Jimenez Rodriguez

Submit Requirements:

Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Luis Jimenez Rodriguez (Gerrit)

unread,

2:47 PM (3 hours ago) 2:47 PM

to Code Review Nudger, Jeffrey Yu, Ben Pastene, LUCI CQ, chromium...@chromium.org, infra-rev...@chromium.org

Attention needed from Ben Pastene and Jeffrey Yu

Luis Jimenez Rodriguez added 3 comments

File recipes/recipe_modules/chromium_tests/steps.py

Line 142, Patchset 2: Args:


      filter_flag: Flag argument used to specify test filters.
      filter_delimiter: The delimiter to use between values when specifying
        test filters.
      repeat_flag: Flag argument used to define how many times to repeat tests.
      retry_limit_flag: Flag argument used to define the upper limit of
        retries.
      run_disabled_flag: Flag argument used to run disabled tests.
      batch_limit_flag: Flag argument used to set how many tests run in a
        given shard.

Ben Pastene . resolved

All of this is already documented in the list of attrs defined at the top of the class. So I don't think this is adding much value.

Luis Jimenez Rodriguez

Done

Line 240, Patchset 2: )

Ben Pastene . resolved

Same. This is unnecessary duplication IMO

Luis Jimenez Rodriguez

Done

Line 835, Patchset 2: target_name: The target name for the test. If not set, defaults to `name`.

Ben Pastene . resolved

I would just get rid of this method's docstring all together, and add this bit into its class-level docstring.

Luis Jimenez Rodriguez

Done

Open in Gerrit

Related details

Attention is currently required from:

Ben Pastene
Jeffrey Yu

Submit Requirements:

Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages