Skip to content

Commit c41bdb4

Browse files
gustavoliraclaude
andcommitted
refactor: address pre-merge review
- Remove the catalog-index mode (script, --catalog-index flag, docs): it can't work until catalog-extending plugins boot (the catalog gap), so shipping it would only produce failures. Re-add when that wiring lands. - Workflow: scope pull_request to branches [main, release-*], matching sibling workflows. - discoverPlugins: guard the per-dir package.json parse so one malformed plugin dir doesn't abort discovery of the rest. - Make the summary say 'skipped (no backend plugins)' for frontend-only input so a green doesn't overstate coverage. - Drop the now-internal-only export on resolveEntryPoint. Kept run() as-is (a clear named 'no shell' helper, likely reused). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 7a86675 commit c41bdb4

5 files changed

Lines changed: 21 additions & 34 deletions

File tree

.github/workflows/native-smoke.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ name: Native Smoke Harness
1313

1414
on:
1515
pull_request:
16+
branches:
17+
- main
18+
- "release-*"
1619
paths:
1720
- "smoke-tests-native/**"
1821
- ".github/workflows/native-smoke.yaml"

smoke-tests-native/README.md

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,11 @@ Requires Node 24 and Yarn 4 (matching the repo's `versions.json` and the sibling
4545
```bash
4646
yarn install
4747

48-
# A) explicit OCI refs for one workspace
4948
cat > dp.yaml <<'YAML'
5049
plugins:
5150
- package: oci://ghcr.io/redhat-developer/rhdh-plugin-export-overlays/<plugin>:<tag>!<name>
5251
YAML
5352
yarn smoke --dynamic-plugins dp.yaml
54-
55-
# B) a whole catalog index image
56-
CATALOG_INDEX_IMAGE=<image> yarn smoke:catalog-index
5753
```
5854

5955
`yarn check` runs `tsc --noEmit`. This is a standalone tool dir, not a

smoke-tests-native/package.json

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
"scripts": {
1313
"build": "esbuild src/native-smoke.ts --bundle --platform=node --format=esm --packages=external --outfile=dist/native-smoke.mjs",
1414
"smoke": "yarn build && node dist/native-smoke.mjs",
15-
"smoke:catalog-index": "yarn build && node dist/native-smoke.mjs --catalog-index",
1615
"tsc:check": "tsc --noEmit",
1716
"check": "yarn tsc:check"
1817
},

smoke-tests-native/src/loader.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,13 @@ export function discoverPlugins(root: string): PluginManifest {
5555
const pkgPath = join(dir, "package.json");
5656
if (!existsSync(pkgPath)) continue;
5757

58-
const pkg = JSON.parse(readFileSync(pkgPath, "utf8"));
58+
let pkg: { name?: string; version?: string; backstage?: { role?: string } };
59+
try {
60+
pkg = JSON.parse(readFileSync(pkgPath, "utf8"));
61+
} catch {
62+
// A malformed package.json in one dir shouldn't abort discovery of the rest.
63+
continue;
64+
}
5965
const role: string = pkg.backstage?.role ?? "";
6066
const item: PluginEntry = {
6167
name: pkg.name ?? entry.name,
@@ -74,7 +80,7 @@ export function discoverPlugins(root: string): PluginManifest {
7480
}
7581

7682
/** Resolve the entry point for a backend plugin package. */
77-
export function resolveEntryPoint(pluginPath: string): string {
83+
function resolveEntryPoint(pluginPath: string): string {
7884
const pkgPath = join(pluginPath, "package.json");
7985
if (!existsSync(pkgPath)) {
8086
throw new Error(`package.json not found in ${pluginPath}`);

smoke-tests-native/src/native-smoke.ts

Lines changed: 10 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@
2424
*
2525
* Usage:
2626
* yarn smoke --dynamic-plugins <dynamic-plugins.yaml> [--out results.json]
27-
* # or, to extract a whole catalog index instead of explicit OCI refs:
28-
* CATALOG_INDEX_IMAGE=<image> yarn smoke:catalog-index
2927
*/
3028

3129
import { execFileSync } from "node:child_process";
@@ -104,29 +102,16 @@ function computeStatus(
104102
return "pass";
105103
}
106104

107-
// Write the dynamic-plugins.yaml the CLI consumes, then extract (the part PR #2231
105+
// Copy the dynamic-plugins.yaml the CLI consumes, then extract (the part PR #2231
108106
// hand-rolled in 694 lines — now one CLI call).
109-
async function extractPlugins(
110-
root: string,
111-
useCatalogIndex: boolean,
112-
dynamicPlugins: string | undefined,
113-
catalogIndexImage: string | undefined,
114-
): Promise<void> {
115-
if (useCatalogIndex) {
116-
// Empty list => CLI extracts everything from CATALOG_INDEX_IMAGE.
117-
await writeFile(join(root, "dynamic-plugins.yaml"), "plugins: []\n");
118-
} else if (dynamicPlugins) {
119-
await copyFile(dynamicPlugins, join(root, "dynamic-plugins.yaml"));
120-
}
107+
async function extractPlugins(root: string, dynamicPlugins: string): Promise<void> {
108+
await copyFile(dynamicPlugins, join(root, "dynamic-plugins.yaml"));
121109
console.log("▶ extracting plugins via CLI…");
122110
// The CLI reads dynamic-plugins.yaml from its CWD, so run it inside `root`
123111
// (where we just wrote the config) and extract into the same dir.
124112
execFileSync(process.execPath, [CLI_BIN, "install", root], {
125113
stdio: "inherit",
126114
cwd: root,
127-
env: catalogIndexImage
128-
? { ...process.env, CATALOG_INDEX_IMAGE: catalogIndexImage }
129-
: process.env,
130115
});
131116
}
132117

@@ -173,20 +158,15 @@ async function main(): Promise<number> {
173158
const { values } = parseArgs({
174159
options: {
175160
"dynamic-plugins": { type: "string" },
176-
"catalog-index": { type: "boolean" },
177161
out: { type: "string" },
178162
},
179163
});
180164

181165
const out = values.out ?? "results.json";
182166
const dynamicPlugins = values["dynamic-plugins"];
183-
const useCatalogIndex = values["catalog-index"] ?? false;
184-
const catalogIndexImage = process.env.CATALOG_INDEX_IMAGE;
185167

186-
if (!useCatalogIndex && !dynamicPlugins) {
187-
console.error(
188-
"Provide --dynamic-plugins <yaml> OR --catalog-index with CATALOG_INDEX_IMAGE set.",
189-
);
168+
if (!dynamicPlugins) {
169+
console.error("Provide --dynamic-plugins <dynamic-plugins.yaml>.");
190170
return 2;
191171
}
192172

@@ -198,7 +178,7 @@ async function main(): Promise<number> {
198178
await mkdir(root, { recursive: true });
199179

200180
try {
201-
await extractPlugins(root, useCatalogIndex, dynamicPlugins, catalogIndexImage);
181+
await extractPlugins(root, dynamicPlugins);
202182

203183
const manifest = discoverPlugins(root);
204184
console.log(
@@ -231,10 +211,13 @@ async function main(): Promise<number> {
231211
};
232212

233213
await writeFile(out, JSON.stringify(report, null, 2));
214+
const startLabel = start.skipped
215+
? "skipped (no backend plugins — frontend bundle presence only)"
216+
: String(start.ok);
234217
console.log(`▶ report → ${out} (status: ${report.status})`);
235218
console.log(
236219
` backend loaded ${report.backend.loaded}/${report.backend.total}, ` +
237-
`start=${start.ok}, frontend ${frontend.valid}/${manifest.frontend.length}`,
220+
`start=${startLabel}, frontend ${frontend.valid}/${manifest.frontend.length}`,
238221
);
239222
return report.status === "pass" ? 0 : 1;
240223
} catch (err) {

0 commit comments

Comments
 (0)