Skip to content

test: remove the system tests from the generator#8150

Draft
danieljbruce wants to merge 40 commits intomainfrom
remove-the-system-tests-generator
Draft

test: remove the system tests from the generator#8150
danieljbruce wants to merge 40 commits intomainfrom
remove-the-system-tests-generator

Conversation

@danieljbruce
Copy link
Copy Markdown
Contributor

@danieljbruce danieljbruce commented May 1, 2026

Description

This change addresses issue #8017. It removes the system tests from the generator which is necessary because the system tests are unhelpful.

Impact

  • Reduces the amount of time the CI will take to run by eliminating the system tests
  • Removes code that is only used by the system tests to eliminate ongoing maintenance

Summary for the reviewer

You'll notice this PR is large, but that is just because of the final npm run baseline command used to generate the baseline file changes. It will be a lot easier to review the changes before running that command at main...ab6beac to evaluate correctness.

These changes include:

  • In the package.json files of the typescript-gapic-generator, don’t run the system tests. Instead just print a message.
  • Remove pack and play from the package.json files where system tests are turned off
  • In core/generator/gapic-generator-typescript/templates/cjs/typescript_gapic/tsconfig.json.njk and core/generator/gapic-generator-typescript/templates/esm/typescript_gapic/tsconfig.json.njk system tests are removed from consideration much like the PR for unit tests (feat: remove unit tests from the generator #8130)
  • system-test folders have been deleted as they only apply to system tests

Copy link
Copy Markdown
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 removes system and unit test configurations, scripts, and baseline files across multiple generator baselines, including asset, bigquery, and compute. The reviewer identified a critical issue where the PR exceeds its stated scope by deleting unit tests and core testing dependencies like mocha, sinon, and c8 alongside the intended system test removals. Furthermore, inconsistencies were found in the compute-esm baseline, where test references were partially removed without corresponding file deletions or configuration updates.

I am having trouble creating individual review comments. Click here to see my feedback.

core/generator/gapic-generator-typescript/baselines/asset-esm/package.json (66-68)

high

The PR title indicates the removal of the system tests generator, but these changes also remove the unit test scripts (test:cjs, test:esm, test), the corresponding unit test files (e.g., esm/test/gapic_asset_service_v1.ts.baseline), and their configuration. Additionally, core testing dependencies like mocha, sinon, and c8 are being removed. If unit tests are intended to remain, they should be preserved along with their dependencies and configuration.

core/generator/gapic-generator-typescript/baselines/compute-esm/.eslintignore.baseline (9)

high

There is an inconsistency in the compute-esm baseline. While unit test references are removed from .eslintignore (line 9) and system test files are deleted, the unit test files themselves are not included in the removal patches, and the package.json and tsconfig.json for this baseline are missing from the PR. This baseline should be updated consistently with the others if the goal is to remove all tests.

@danieljbruce danieljbruce changed the title Remove the system tests generator test: remove the system tests from the generator May 1, 2026
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