Skip to content
Closed
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
11 changes: 8 additions & 3 deletions src/TaskRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { RetryingExecutionStrategy } from "./strategies/RetryingExecutionStrateg
import { Plugin } from "./contracts/Plugin.js";
import { PluginManager } from "./PluginManager.js";
import { DryRunExecutionStrategy } from "./strategies/DryRunExecutionStrategy.js";
import { filterTasks } from "./utils/TaskFilter.js";

const MERMAID_ID_REGEX = /[^a-zA-Z0-9_-]/g;

Expand Down Expand Up @@ -176,9 +177,13 @@ export class TaskRunner<TContext> {
// Initialize plugins
await this.pluginManager.initialize();

const tasksToRun = config?.filter
? filterTasks(steps, config.filter)
: steps;

// Validate the task graph before execution
const taskGraph: TaskGraph = {
tasks: steps.map((step) => ({
tasks: tasksToRun.map((step) => ({
id: step.name,
dependencies: step.dependencies ?? [],
})),
Expand Down Expand Up @@ -210,13 +215,13 @@ export class TaskRunner<TContext> {
if (config?.timeout !== undefined) {
return this.executeWithTimeout(
executor,
steps,
tasksToRun,
config.timeout,
config.signal
);
}

return executor.execute(steps, config?.signal);
return executor.execute(tasksToRun, config?.signal);
}

/**
Expand Down
6 changes: 6 additions & 0 deletions src/TaskRunnerExecutionConfig.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { TaskFilterConfig } from "./contracts/TaskFilterConfig.js";

/**
* Configuration options for TaskRunner execution.
*/
Expand All @@ -20,4 +22,8 @@ export interface TaskRunnerExecutionConfig {
* If undefined, all ready tasks will be run in parallel.
*/
concurrency?: number;
/**
* Optional filter configuration to run only a subset of tasks.
*/
filter?: TaskFilterConfig;
}
2 changes: 2 additions & 0 deletions src/TaskStep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ export interface TaskStep<TContext> {
name: string;
/** An optional list of task names that must complete successfully before this step can run. */
dependencies?: string[];
/** Optional tags to categorize and filter this task. */
tags?: string[];
/** Optional retry configuration for the task. */
retry?: TaskRetryConfig;
/** Optional loop configuration for the task. */
Expand Down
12 changes: 12 additions & 0 deletions src/contracts/TaskFilterConfig.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export interface TaskFilterConfig {
/** Include tasks with any of these tags. */
includeTags?: string[];
/** Exclude tasks with any of these tags. */
excludeTags?: string[];
/** Include tasks with these specific names. */
includeNames?: string[];
/** Exclude tasks with these specific names. */
excludeNames?: string[];
/** If true, recursively includes dependencies of selected tasks. */
includeDependencies?: boolean;
}
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ export { TaskStateManager } from "./TaskStateManager.js";
export { TaskGraphValidationError } from "./TaskGraphValidationError.js";
export { StandardExecutionStrategy } from "./strategies/StandardExecutionStrategy.js";
export { RetryingExecutionStrategy } from "./strategies/RetryingExecutionStrategy.js";
export { filterTasks } from "./utils/TaskFilter.js";
export type { IExecutionStrategy } from "./strategies/IExecutionStrategy.js";
export type { TaskRetryConfig } from "./contracts/TaskRetryConfig.js";
export type { TaskFilterConfig } from "./contracts/TaskFilterConfig.js";
export type { TaskStep } from "./TaskStep.js";
export type { TaskResult } from "./TaskResult.js";
export type { TaskStatus } from "./TaskStatus.js";
94 changes: 94 additions & 0 deletions src/utils/TaskFilter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import { TaskStep } from "../TaskStep.js";
import { TaskFilterConfig } from "../contracts/TaskFilterConfig.js";

/**
* Filters an array of TaskSteps based on a TaskFilterConfig.
* @param steps The steps to filter.
* @param config The filtering configuration.
* @returns A new array of filtered TaskSteps.
*/
export function filterTasks<T>(

Check failure on line 10 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 30 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Re&open=AZ1Q51mYTm5LOco2D7Re&pullRequest=244
steps: TaskStep<T>[],
config: TaskFilterConfig
): TaskStep<T>[] {
const {
includeTags = [],
excludeTags = [],
includeNames = [],
excludeNames = [],
includeDependencies = false,
} = config;

const hasInclusions = includeTags.length > 0 || includeNames.length > 0;

// 1. Initial Filtering
const selectedTasks = new Set<string>();

for (let i = 0; i < steps.length; i++) {
const step = steps[i]!;

Check warning on line 28 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Rg&open=AZ1Q51mYTm5LOco2D7Rg&pullRequest=244

// Evaluate exclusions first
const isExcludedByName = excludeNames.includes(step.name);
const isExcludedByTag = step.tags?.some(tag => excludeTags.includes(tag)) ?? false;

if (isExcludedByName || isExcludedByTag) {
continue;
}

if (!hasInclusions) {
selectedTasks.add(step.name);
continue;
}

// Evaluate inclusions
const isIncludedByName = includeNames.includes(step.name);
const isIncludedByTag = step.tags?.some(tag => includeTags.includes(tag)) ?? false;

if (isIncludedByName || isIncludedByTag) {
selectedTasks.add(step.name);
}
}

Check warning on line 50 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Expected a `for-of` loop instead of a `for` loop with this simple iteration.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Rf&open=AZ1Q51mYTm5LOco2D7Rf&pullRequest=244

// 2. Resolve Dependencies
/* v8 ignore start */
if (includeDependencies) {
const stepMap = new Map<string, TaskStep<T>>();
for (let i = 0; i < steps.length; i++) {
stepMap.set(steps[i]!.name, steps[i]!);

Check warning on line 57 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Ri&open=AZ1Q51mYTm5LOco2D7Ri&pullRequest=244

Check warning on line 57 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Rj&open=AZ1Q51mYTm5LOco2D7Rj&pullRequest=244
}

Check warning on line 58 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Expected a `for-of` loop instead of a `for` loop with this simple iteration.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Rh&open=AZ1Q51mYTm5LOco2D7Rh&pullRequest=244

const queue = Array.from(selectedTasks);
let head = 0;

while (head < queue.length) {
const currentName = queue[head]!;

Check warning on line 64 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Rk&open=AZ1Q51mYTm5LOco2D7Rk&pullRequest=244
head++;

const step = stepMap.get(currentName);
/* v8 ignore next 1 */
if (!step) continue;
if (step.dependencies) {
for (let i = 0; i < step.dependencies.length; i++) {
const depName = step.dependencies[i]!;

Check warning on line 72 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Rm&open=AZ1Q51mYTm5LOco2D7Rm&pullRequest=244
if (!selectedTasks.has(depName)) {
selectedTasks.add(depName);
queue.push(depName);
}
}

Check warning on line 77 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Expected a `for-of` loop instead of a `for` loop with this simple iteration.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Rl&open=AZ1Q51mYTm5LOco2D7Rl&pullRequest=244
}
}
}

/* v8 ignore stop */

// 3. Return Filtered Array
const result: TaskStep<T>[] = [];
for (let i = 0; i < steps.length; i++) {
const step = steps[i]!;

Check warning on line 87 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

This assertion is unnecessary since it does not change the type of the expression.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Ro&open=AZ1Q51mYTm5LOco2D7Ro&pullRequest=244
if (selectedTasks.has(step.name)) {
result.push(step);
}
}

Check warning on line 91 in src/utils/TaskFilter.ts

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Expected a `for-of` loop instead of a `for` loop with this simple iteration.

See more on https://sonarcloud.io/project/issues?id=thalesraymond_task-runner&issues=AZ1Q51mYTm5LOco2D7Rn&open=AZ1Q51mYTm5LOco2D7Rn&pullRequest=244
Comment on lines +14 to +91
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The filterTasks function can be improved in terms of performance, correctness, and adherence to TypeScript best practices:

  1. Performance: Use Set objects for includeTags, excludeTags, includeNames, and excludeNames to ensure O(1) lookup time. The current implementation uses Array.prototype.includes inside loops, leading to O(N * M) complexity.
  2. Correctness: The dependency resolution phase should respect the exclusion filters. Currently, dependencies are added to the execution set even if they were explicitly excluded by name or tag. By respecting exclusions here, the TaskRunner's graph validation will correctly identify missing required dependencies.
  3. Maintainability: Use for...of loops instead of indexed for loops to avoid the non-null assertion operator (!), adhering to the project's general rules.
  const includeTagsSet = new Set(config.includeTags ?? []);
  const excludeTagsSet = new Set(config.excludeTags ?? []);
  const includeNamesSet = new Set(config.includeNames ?? []);
  const excludeNamesSet = new Set(config.excludeNames ?? []);
  const { includeDependencies = false } = config;

  const hasInclusions = includeTagsSet.size > 0 || includeNamesSet.size > 0;

  // 1. Initial Filtering
  const selectedTasks = new Set<string>();

  for (const step of steps) {
    // Evaluate exclusions first
    const isExcludedByName = excludeNamesSet.has(step.name);
    const isExcludedByTag = step.tags?.some((tag) => excludeTagsSet.has(tag)) ?? false;

    if (isExcludedByName || isExcludedByTag) {
      continue;
    }

    if (!hasInclusions) {
      selectedTasks.add(step.name);
      continue;
    }

    // Evaluate inclusions
    const isIncludedByName = includeNamesSet.has(step.name);
    const isIncludedByTag = step.tags?.some((tag) => includeTagsSet.has(tag)) ?? false;

    if (isIncludedByName || isIncludedByTag) {
      selectedTasks.add(step.name);
    }
  }

  // 2. Resolve Dependencies
  if (includeDependencies) {
    const stepMap = new Map<string, TaskStep<T>>();
    for (const step of steps) {
      stepMap.set(step.name, step);
    }

    const queue = Array.from(selectedTasks);
    for (let i = 0; i < queue.length; i++) {
      const currentName = queue[i];
      if (currentName === undefined) continue;

      const step = stepMap.get(currentName);
      if (!step || !step.dependencies) continue;

      for (const depName of step.dependencies) {
        if (!selectedTasks.has(depName)) {
          const depStep = stepMap.get(depName);
          const isExcludedByName = excludeNamesSet.has(depName);
          const isExcludedByTag = depStep?.tags?.some((tag) => excludeTagsSet.has(tag)) ?? false;

          if (!isExcludedByName && !isExcludedByTag) {
            selectedTasks.add(depName);
            queue.push(depName);
          }
        }
      }
    }
  }

  // 3. Return Filtered Array
  const result: TaskStep<T>[] = [];
  for (const step of steps) {
    if (selectedTasks.has(step.name)) {
      result.push(step);
    }
  }
References
  1. When frequent lookups of items by a specific key are required, use a Map (or dictionary/hash table) to store the items, indexed by that key, to ensure O(1) access time.
  2. In TypeScript, avoid using the non-null assertion operator (!). Instead, use explicit checks or safe fallbacks like the nullish coalescing operator (??) to handle potentially null or undefined values. This improves robustness against future changes that might break assumptions about the value's existence.


return result;
}
126 changes: 126 additions & 0 deletions tests/TaskFilter.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { describe, it, expect } from "vitest";
import { filterTasks } from "../src/utils/TaskFilter.js";
import { TaskStep } from "../src/TaskStep.js";
import { TaskStatus } from "../src/TaskStatus.js";

describe("filterTasks", () => {
const defaultTaskRun = async () => ({ status: "success" as TaskStatus });
const tasks: TaskStep<unknown>[] = [
{ name: "taskA", tags: ["backend", "core"], run: defaultTaskRun },
{ name: "taskB", tags: ["backend"], dependencies: ["taskA"], run: defaultTaskRun },
{ name: "taskC", tags: ["frontend"], run: defaultTaskRun },
{ name: "taskD", tags: ["frontend", "ui"], dependencies: ["taskC"], run: defaultTaskRun },
{ name: "taskE", dependencies: ["taskB", "taskD"], run: defaultTaskRun },
];

it("should return all tasks if no inclusions or exclusions are provided", () => {
const filtered = filterTasks(tasks, {});
expect(filtered).toHaveLength(5);
expect(filtered.map(t => t.name)).toEqual(["taskA", "taskB", "taskC", "taskD", "taskE"]);
});

it("should include tasks by specific names", () => {
const filtered = filterTasks(tasks, { includeNames: ["taskA", "taskC"] });
expect(filtered).toHaveLength(2);
expect(filtered.map(t => t.name)).toEqual(["taskA", "taskC"]);
});

it("should include tasks by tags", () => {
const filtered = filterTasks(tasks, { includeTags: ["backend"] });
expect(filtered).toHaveLength(2);
expect(filtered.map(t => t.name)).toEqual(["taskA", "taskB"]);
});

it("should exclude tasks by specific names", () => {
const filtered = filterTasks(tasks, { excludeNames: ["taskB", "taskD", "taskE"] });
expect(filtered).toHaveLength(2);
expect(filtered.map(t => t.name)).toEqual(["taskA", "taskC"]);
});

it("should exclude tasks by tags", () => {
const filtered = filterTasks(tasks, { excludeTags: ["backend"] });
expect(filtered).toHaveLength(3);
expect(filtered.map(t => t.name)).toEqual(["taskC", "taskD", "taskE"]);
});

it("should handle combinations of inclusions and exclusions", () => {
// Include backend tasks but exclude taskB
const filtered = filterTasks(tasks, { includeTags: ["backend"], excludeNames: ["taskB"] });
expect(filtered).toHaveLength(1);
expect(filtered.map(t => t.name)).toEqual(["taskA"]);
});

it("should include dependencies recursively when includeDependencies is true", () => {
const filtered = filterTasks(tasks, { includeNames: ["taskE"], includeDependencies: true });
// taskE depends on taskB and taskD
// taskB depends on taskA
// taskD depends on taskC
// So all tasks should be included
expect(filtered).toHaveLength(5);
expect(filtered.map(t => t.name).sort()).toEqual(["taskA", "taskB", "taskC", "taskD", "taskE"].sort());
});

it("should not include dependencies when includeDependencies is false", () => {
const filtered = filterTasks(tasks, { includeNames: ["taskE"], includeDependencies: false });
expect(filtered).toHaveLength(1);
expect(filtered.map(t => t.name)).toEqual(["taskE"]);
});

it("should handle exclusions even if includeDependencies pulls them in", () => {
// Current implementation: Initial filtering applies inclusions and exclusions.
// If includeDependencies is true, it recursively adds dependencies of *initially selected* tasks,
// overriding exclusions for those dependencies if they weren't in the initial set.
// This test verifies the current behavior, if we want strict exclusion we might need to modify filterTasks.
// But currently, the design is: dependencies of included tasks are included.
const filtered = filterTasks(tasks, { includeNames: ["taskB"], includeDependencies: true, excludeNames: ["taskA"] });
expect(filtered.map(t => t.name).sort()).toEqual(["taskA", "taskB"].sort());
});
Comment on lines +69 to +77
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This test case currently expects that includeDependencies overrides explicit exclusions. However, for consistency and predictability, exclusions should generally take precedence. If a task is explicitly excluded, it should not be pulled back into the execution set. Consider updating the implementation to respect exclusions during dependency resolution and updating this test to verify that behavior.

  it("should respect exclusions even if includeDependencies is true", () => {
    // If taskB is included and depends on taskA, but taskA is excluded,
    // taskA should NOT be pulled in.
    const filtered = filterTasks(tasks, { includeNames: ["taskB"], includeDependencies: true, excludeNames: ["taskA"] });
    expect(filtered.map(t => t.name)).toEqual(["taskB"]);
  });


it("should include both name inclusions and tag inclusions", () => {
const filtered = filterTasks(tasks, { includeNames: ["taskC"], includeTags: ["core"] });
expect(filtered).toHaveLength(2);
expect(filtered.map(t => t.name)).toEqual(["taskA", "taskC"]);
});

it("should ignore dependencies that do not exist in the steps array when resolving dependencies", () => {
const missingDepTasks: TaskStep<unknown>[] = [
{ name: "task1", dependencies: ["nonExistent"], run: defaultTaskRun },
];
// Force resolving an undefined step from stepMap
const filtered = filterTasks(missingDepTasks, { includeNames: ["task1", "nonExistent"], includeDependencies: true });
expect(filtered).toHaveLength(1);
expect(filtered.map(t => t.name)).toEqual(["task1"]);
});

it("should handle undefined dependencies during resolution", () => {
const noDepsTasks: TaskStep<unknown>[] = [
{ name: "task1", run: defaultTaskRun },
];
const filtered = filterTasks(noDepsTasks, { includeNames: ["task1"], includeDependencies: true });
expect(filtered).toHaveLength(1);
expect(filtered.map(t => t.name)).toEqual(["task1"]);
});

it("should ignore tasks if they somehow have no tags and excludeTags is passed", () => {
const missingDepTasks: TaskStep<unknown>[] = [
{ name: "task1", run: defaultTaskRun },
];
const filtered = filterTasks(missingDepTasks, { excludeTags: ["sometag"] });
expect(filtered).toHaveLength(1);
expect(filtered.map(t => t.name)).toEqual(["task1"]);
});

it("should ignore tasks if they somehow have no tags and includeTags is passed", () => {
const missingDepTasks: TaskStep<unknown>[] = [
{ name: "task1", run: defaultTaskRun },
];
const filtered = filterTasks(missingDepTasks, { includeTags: ["sometag"] });
expect(filtered).toHaveLength(0);
});

it("should not crash if includeTags is uninitialized somehow", () => {
// Calling with empty object defaults covered, now try forcing undefined
const filtered = filterTasks(tasks, { includeTags: undefined, excludeTags: undefined });
expect(filtered).toHaveLength(5);
});
});
Loading
Loading