Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
105 changes: 69 additions & 36 deletions src/services/execution-plan.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as path from 'node:path';
import {
getFlowsToRunInSequence,
isFlowFile,
isWorkspaceConfigFile,
processDependencies,
readDirectory,
readTestYamlFileAsJson,
Expand Down Expand Up @@ -198,16 +199,18 @@ function extractDeviceCloudOverrides(
/**
* Generate execution plan for a single flow file
* @param normalizedInput - Normalized path to the flow file
* @param configFile - Optional custom config file path
* @param resolvedConfigFile - Optional absolute path to a custom config file
* @returns Execution plan for the single file with dependencies
*/
async function planSingleFile(
normalizedInput: string,
configFile?: string,
resolvedConfigFile?: string,
): Promise<IExecutionPlan> {
const inputBasename = path.basename(normalizedInput);
if (
normalizedInput.endsWith('config.yaml') ||
normalizedInput.endsWith('config.yml')
inputBasename === 'config.yaml' ||
inputBasename === 'config.yml' ||
isWorkspaceConfigFile(normalizedInput)
) {
throw new Error(
'If using config.yaml, pass the workspace folder path, not the config file or a custom path via --config',
Expand All @@ -224,13 +227,14 @@ async function planSingleFile(
}

let workspaceConfig: IWorkspaceConfig | undefined;
if (configFile) {
const configFilePath = path.resolve(process.cwd(), configFile);
if (!fs.existsSync(configFilePath)) {
throw new Error(`Config file does not exist: ${configFilePath}`);
if (resolvedConfigFile) {
if (!fs.existsSync(resolvedConfigFile)) {
throw new Error(`Config file does not exist: ${resolvedConfigFile}`);
}

workspaceConfig = readYamlFileAsJson(configFilePath) as IWorkspaceConfig;
workspaceConfig = readYamlFileAsJson(
resolvedConfigFile,
) as IWorkspaceConfig;
}

const checkedDependancies = await checkDependencies(normalizedInput);
Expand All @@ -249,17 +253,32 @@ async function planSingleFile(
* @param workspaceConfig - Workspace configuration containing flow globs
* @param normalizedInput - Normalized path to the workspace directory
* @param unfilteredFlowFiles - List of all discovered flow files
* @param configFile - Optional custom config file path
* @param resolvedConfigFile - Optional absolute path to a custom config file
* @param excludeFlows - --exclude-flows patterns to re-apply to glob matches
* @returns Filtered list of flow file paths matching the globs
*/
async function applyFlowGlobs(
workspaceConfig: IWorkspaceConfig,
normalizedInput: string,
unfilteredFlowFiles: string[],
configFile?: string,
resolvedConfigFile?: string,
excludeFlows?: string[],
): Promise<string[]> {
// Both branches compare absolute paths, so `--config ./x.yml` and
// `--config /abs/x.yml` behave identically, and a same-named file in a
// sibling directory is never mistaken for the active config.
const activeConfig = resolvedConfigFile
? path.normalize(resolvedConfigFile)
: undefined;
const isExcludedConfig = (absolutePath: string): boolean => {
const base = path.basename(absolutePath);
if (base === 'config.yaml' || base === 'config.yml') return true;
return (
activeConfig !== undefined &&
path.normalize(absolutePath) === activeConfig
);
};

if (workspaceConfig.flows) {
const globs = workspaceConfig.flows.map((g) => g);
// fs.globSync lands in Node 22; the CLI's `engines.node` already requires it.
Expand All @@ -273,31 +292,19 @@ async function applyFlowGlobs(
}
});

// Resolve before filtering: glob matches are relative to normalizedInput,
// so comparing them against the config path only ever worked when the
// config happened to sit at the workspace root.
const globbedFlowFiles = matchedFiles
.filter((file: string) => {
if (file === 'config.yaml' || file === 'config.yml') return false;
if (configFile && file === path.basename(configFile)) return false;
if (!file.endsWith('.yaml') && !file.endsWith('.yml')) return false;
const pathParts = file.split(path.sep);
for (const part of pathParts) {
if (part.endsWith('.app')) return false;
}

return true;
})
.map((file) => path.resolve(normalizedInput, file));
.map((file) => path.resolve(normalizedInput, file))
.filter((file) => !isExcludedConfig(file) && isFlowFile(file));

// Re-globbing from disk bypasses the earlier --exclude-flows filter, so
// re-apply it here or excluded flows sneak back in via `flows:` globs.
return filterFlowFiles(globbedFlowFiles, excludeFlows);
}

return unfilteredFlowFiles.filter(
(file) =>
!file.endsWith('config.yaml') &&
!file.endsWith('config.yml') &&
(!configFile || !file.endsWith(configFile)),
);
return unfilteredFlowFiles.filter((file) => !isExcludedConfig(file));
}

/**
Expand Down Expand Up @@ -382,6 +389,9 @@ export async function plan(options: PlanOptions): Promise<IExecutionPlan> {
} = options;
const normalizedInput = path.normalize(input);
const flowMetadata: Record<string, Record<string, unknown>> = {};
const resolvedConfigFile = configFile
? path.resolve(process.cwd(), configFile)
: undefined;

if (!fs.existsSync(normalizedInput)) {
throw new Error(
Expand All @@ -390,7 +400,7 @@ export async function plan(options: PlanOptions): Promise<IExecutionPlan> {
}

if (fs.lstatSync(normalizedInput).isFile()) {
return planSingleFile(normalizedInput, configFile);
return planSingleFile(normalizedInput, resolvedConfigFile);
}

let unfilteredFlowFiles = await readDirectory(normalizedInput, isFlowFile);
Expand All @@ -405,13 +415,14 @@ export async function plan(options: PlanOptions): Promise<IExecutionPlan> {
unfilteredFlowFiles = filterFlowFiles(unfilteredFlowFiles, excludeFlows);

let workspaceConfig: IWorkspaceConfig;
if (configFile) {
const configFilePath = path.resolve(process.cwd(), configFile);
if (!fs.existsSync(configFilePath)) {
throw new Error(`Config file does not exist: ${configFilePath}`);
if (resolvedConfigFile) {
if (!fs.existsSync(resolvedConfigFile)) {
throw new Error(`Config file does not exist: ${resolvedConfigFile}`);
}

workspaceConfig = readYamlFileAsJson(configFilePath) as IWorkspaceConfig;
workspaceConfig = readYamlFileAsJson(
resolvedConfigFile,
) as IWorkspaceConfig;
} else {
workspaceConfig = getWorkspaceConfig(normalizedInput, unfilteredFlowFiles);
}
Expand All @@ -420,10 +431,32 @@ export async function plan(options: PlanOptions): Promise<IExecutionPlan> {
workspaceConfig,
normalizedInput,
unfilteredFlowFiles,
configFile,
resolvedConfigFile,
excludeFlows,
);

// The exclusions above are filename-based: they only catch
// `config.yaml`/`config.yml` and the active --config target. Any other
// workspace config sharing the folder (a second CI workflow's, say) is still
// on the list and would be parsed as a flow, so drop config-shaped files by
// shape — see dcd-cli#99.
const configShapedFiles = new Set(
unfilteredFlowFiles.filter((file) => isWorkspaceConfigFile(file)),
);
if (configShapedFiles.size > 0) {
if (debug) {
console.log(
`[DEBUG] Skipping ${configShapedFiles.size} workspace config file(s): ${[
...configShapedFiles,
].join(', ')}`,
);
}

unfilteredFlowFiles = unfilteredFlowFiles.filter(
(file) => !configShapedFiles.has(file),
);
}

if (unfilteredFlowFiles.length === 0) {
const error = workspaceConfig.flows
? new Error(
Expand Down
47 changes: 47 additions & 0 deletions src/services/execution-plan.utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,53 @@ export function isFlowFile(filePath: string): boolean {
return filePath.endsWith('.yaml') || filePath.endsWith('.yml');
}

/**
* Top-level keys that only ever appear in a workspace config (see
* IWorkspaceConfig in execution-plan.service.ts). Deliberately excludes keys
* Maestro also allows in flow front matter — appId, name, tags, env,
* onFlowStart, onFlowComplete, jsEngine.
*/
const WORKSPACE_CONFIG_KEYS = new Set([
'excludeTags',
'executionOrder',
'flows',
'includeTags',
'local',
'notifications',
'platform',
]);

/**
* True when a YAML file is a workspace config rather than a runnable flow.
*
* A flow is either `front matter --- steps` or a bare steps array; a
* single-document top-level map carrying workspace-config keys is neither, and
* left in the flow list it blows up processDependencies with "Expected an array
* of steps". Detection is by shape, not filename, so several named configs can
* coexist in one folder (dcd-cli#99). Requiring a recognised config key — not
* just "single document, top-level map" — keeps a flow that is merely *missing*
* its `---` separator loud rather than silently dropped.
*
* @param filePath - Path to the YAML file to classify
* @returns Whether the file is a workspace config rather than a flow
*/
export function isWorkspaceConfigFile(filePath: string): boolean {
let parsed;
try {
parsed = readTestYamlFileAsJson(filePath);
} catch {
// Unparseable — leave it in the flow list so the existing error path reports it.
return false;
}

const { config, testSteps } = parsed;
if (config !== null) return false; // has `---` front matter → flow
if (Array.isArray(testSteps)) return false; // bare steps array → flow
if (!testSteps || typeof testSteps !== 'object') return false;

return Object.keys(testSteps).some((key) => WORKSPACE_CONFIG_KEYS.has(key));
}

export const readYamlFileAsJson = (filePath: string) => {
try {
const normalizedPath = path.normalize(filePath);
Expand Down
121 changes: 121 additions & 0 deletions test/integration/cloud.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,127 @@ appId: com.example.app
});
});

// Regression cover for dcd-cli#99: config files were excluded from flow
// discovery by *filename* (literal config.yaml/config.yml plus the exact
// --config target), so a second workspace config sharing the folder was
// parsed as a flow and blew up with "Expected an array of steps".
// These must pass the flow *directory* — the --config tests below pass a
// single file, which short-circuits into planSingleFile and never reaches
// flow discovery.
describe('workspace config discovery', () => {
let workspaceDir: string;
let buildConfig: string;
let updateConfig: string;
let globConfig: string;

before(() => {
workspaceDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dcd-test-ws-'));

fs.writeFileSync(
path.join(workspaceDir, 'flow.yaml'),
`appId: com.example.app
---
- launchApp
- tapOn: "Login"
`,
);

// A genuine flow whose name merely ends in "config.yaml". The old
// unanchored endsWith() dropped it; it must run.
fs.writeFileSync(
path.join(workspaceDir, 'smoke-config.yaml'),
`appId: com.example.app
name: smoke-config
---
- launchApp
`,
);

// Two sibling workspace configs with custom names, as a folder serving
// several CI workflows would have. Neither is a flow.
buildConfig = path.join(workspaceDir, 'config_build.yml');
fs.writeFileSync(
buildConfig,
`flows:
- ./**/*.yaml
includeTags:
- build
`,
);

// Deliberately carries no tag or flow filtering, so when it is the active
// --config it can't mask the bug by filtering its sibling away first.
updateConfig = path.join(workspaceDir, 'config_update.yml');
fs.writeFileSync(
updateConfig,
`platform:
android:
disableAnimations: true
`,
);

globConfig = path.join(workspaceDir, 'config_glob.yml');
fs.writeFileSync(
globConfig,
`flows:
- ./**/*.yaml
- ./**/*.yml
`,
);
});

after(() => {
if (fs.existsSync(workspaceDir)) {
fs.rmSync(workspaceDir, { force: true, recursive: true });
}
});

it('should ignore sibling config files not named in --config', async () => {
const command = `${CLI} cloud ${androidAppFile} "${workspaceDir}" --api-key ${mockApiKey} --api-url ${mockApiUrl} --config "${updateConfig}" --dry-run`;

const { stdout } = await exec(command, { timeout: 15_000 });
expect(stdout).to.include('The following tests would have been run');
expect(stdout).to.include('flow.yaml');
expect(stdout).to.not.include('config_build.yml');
expect(stdout).to.not.include('config_glob.yml');
expect(stdout).to.not.include('Expected an array of steps');
});

it('should ignore config-shaped files when no --config is passed', async () => {
const command = `${CLI} cloud ${androidAppFile} "${workspaceDir}" --api-key ${mockApiKey} --api-url ${mockApiUrl} --debug --dry-run`;

const { stdout } = await exec(command, { timeout: 15_000 });
expect(stdout).to.include('flow.yaml');
expect(stdout).to.include('[DEBUG] Skipping 3 workspace config file(s)');
expect(stdout).to.not.include('Expected an array of steps');
});

it('should ignore config-shaped files matched by a flows glob', async () => {
const command = `${CLI} cloud ${androidAppFile} "${workspaceDir}" --api-key ${mockApiKey} --api-url ${mockApiUrl} --config "${globConfig}" --dry-run`;

const { stdout } = await exec(command, { timeout: 15_000 });
expect(stdout).to.include('flow.yaml');
expect(stdout).to.not.include('config_build.yml');
expect(stdout).to.not.include('config_update.yml');
expect(stdout).to.not.include('Expected an array of steps');
});

it('should run a flow whose filename merely ends in config.yaml', async () => {
const command = `${CLI} cloud ${androidAppFile} "${workspaceDir}" --api-key ${mockApiKey} --api-url ${mockApiUrl} --config "${updateConfig}" --dry-run`;

const { stdout } = await exec(command, { timeout: 15_000 });
expect(stdout).to.include('smoke-config.yaml');
});

it('should reject a custom-named config passed as the flow input', async () => {
const command = `${CLI} cloud ${androidAppFile} "${buildConfig}" --api-key ${mockApiKey} --api-url ${mockApiUrl} --dry-run`;

const { output } = await runExpectingFailure(command);
expect(output).to.include('pass the workspace folder path');
expect(output).to.not.include('Expected an array of steps');
});
});

describe('file and binary management', () => {
it('should support app binary ID instead of file', async () => {
const command = `${CLI} cloud --app-binary-id test-binary-123 ${testFlowFile} --api-key ${mockApiKey} --api-url ${mockApiUrl} --dry-run`;
Expand Down
Loading