Skip to content

fix: bug in system test deletion logic#6845

Merged
sofisl merged 8 commits intomainfrom
fixSystemTests
Oct 24, 2025
Merged

fix: bug in system test deletion logic#6845
sofisl merged 8 commits intomainfrom
fixSystemTests

Conversation

@sofisl
Copy link
Contributor

@sofisl sofisl commented Oct 24, 2025

follow up to #6795. Because the logic spliced elements from the array, we were skipping some elements, causing the array to skip removing the other non-default-version system tests. I fixed this by iterating the array backwards.

To confirm the fix, I changed the test that was testing this to point to the actual troublesome library in #6842.

The only logic to check here is packages/gapic-node-processing/src/combine-libraries.ts and packages/gapic-node-processing/test/combine-libraries.test.ts

Edit: turns out that our tests in packages/gapic-node-processing/test/generate-index.test.ts were actually depending on generating the other library (which is wrong), which led me to discover we weren't setting up test fixtures correctly in packages/gapic-node-processing/test/generate-index.test.ts. So I fixed which library we were generating there so tests could run without accidentally relying on the tasks library to be generated in packages/gapic-node-processing/test/combine-libraries.test.ts.

@sofisl sofisl requested review from a team and yoshi-approver as code owners October 24, 2025 19:18
@snippet-bot
Copy link

snippet-bot bot commented Oct 24, 2025

Here is the summary of changes.

You are about to add 50 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@sofisl sofisl merged commit 468c233 into main Oct 24, 2025
10 of 11 checks passed
@sofisl sofisl deleted the fixSystemTests branch October 24, 2025 20:54
@release-please release-please bot mentioned this pull request Oct 24, 2025
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.

2 participants