googleapis / googleapis/google-cloud-node
bug(gapic-node-processing): setOnlyDefaultSystemTests incorrectly matches substring on absolute path
- Lingua principale
- TypeScript
- Stelle
- 3.2k
- Fork
- 713
- Merge medio
- 2g 9h
- PR unite (30g)
- 104
Descrizione
### Description
In `gapic-node-processing/src/combine-libraries.ts`, `setOnlyDefaultSystemTests()` filters out sample test fixtures from non-default versions when combining multi-version libraries (e.g., `google-iam` with `v2` and `v3`).
It currently uses `.includes(defaultVersion)` on the full absolute file path:
```typescript
function setOnlyDefaultSystemTests(defaultVersion: string, filePaths: FilePaths[]) {
const systemTestRegex = new RegExp('system-test/fixtures/sample/src');
for (let i = filePaths.length - 1; i >= 0; i--) {
const filePathObj = filePaths[i];
const normalizedPath = filePathObj.filePath.replace(/\\/g, '/');
if (systemTestRegex.test(normalizedPath) &&
!normalizedPath.includes(defaultVersion)) {
filePaths.splice(i, 1);
}
}
}
```
### Bug Behavior
If the workspace or temporary directory path where Librarian/generator runs happens to contain the substring `defaultVersion` (for example, `/tmp/upgrade-nodejs-v2Mp8W` containing `"v2"`), `normalizedPath.includes("v2")` evaluates to `true` for all API versions (including `v3`).
As a result:
1. `!normalizedPath.includes(defaultVersion)` evaluates to `false`.
2. `v3` fixture files are not filtered out.
3. `v3` sample fixtures overwrite `v2` sample fixtures, generating unintended client diffs (e.g., `packages/google-iam/system-test/fixtures/sample/src/index.ts` exporting `AccessPoliciesClient` instead of `PoliciesClient`).
### Reference PR & CI Breakage
This issue was observed during Librarian version upgrade in:
- **Pull Request:** https://github.com/googleapis/google-cloud-node/pull/9340
- **Affected Commit:** `559da2d4` (where `mktemp -d` generated `/tmp/upgrade-nodejs-v2Mp8W`)
- **CI Failure:** https://github.com/googleapis/google-cloud-node/actions/runs/34998266744/job/104479939975?pr=9340 (clean CI runner without `v2` in path did not generate the `v3` diffs, causing a `git diff` discrepancy).
### Suggested Fix
Match the version directory boundary specifically rather than performing an unconstrained substring check across the absolute path:
```typescript
const versionDirRegex = new RegExp(`(^|/)${defaultVersion}(/|$)`);
if (systemTestRegex.test(normalizedPath) && !versionDirRegex.test(normalizedPath)) {
filePaths.splice(i, 1);
}
```
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Iniziate in gapic-node-processing/src/combine-libraries.ts e ispezionate setOnlyDefaultSystemTests(), concentrandovi sul modo in cui vengono confrontati i percorsi assoluti normalizzati. Riproducete o esaminate il caso di fixture v2/v3 descritto nell’issue, quindi verificate che il filtraggio delle versioni non sia influenzato da nomi di directory non correlati come /tmp/upgrade-nodejs-v2Mp8W e che l’output della fixture generata non produca più diff indesiderati.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- node.js, typescript
- Ambito
- build-system, tooling
- Tipo di issue
- Bug
- Difficoltà
- 2/5
- Tempo stimato
- 1-3 ore
- Stato di attività
- Attiva
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 88/100