Skip missing: fixing a bug the crept up where npmrunner is still logging about missing scripts (#562)

* fixed reporter to not log target status when it should not run at all

* Change files

change/@lage-run-scheduler-5c6f3fc9-4985-4180-8c70-042048f02119.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fixed reporter to not log target status when it should not run at all",
+  "packageName": "@lage-run/scheduler",
+  "email": "kchau@microsoft.com",
+  "dependentChangeType": "patch"
+}

packages/e2e-tests/src/__snapshots__/info.test.ts.snap

@@ -12,7 +12,7 @@ exports[`info command basic info test case 1`] = `
       "a#build",
     ],
     "dependents": [],
-    "hash": "47c840184f599036508d2fe6970e53e5241ac73f",
+    "hash": "c6c367cb4149d14853f7574c42edec1e6e24658c",
     "id": "a#test",
     "label": "a - test",
     "options": {},
@@ -31,7 +31,7 @@ exports[`info command basic info test case 1`] = `
       "b#build",
     ],
     "dependents": [],
-    "hash": "5a9536691284014e65dd9fffd8a60552c0a5d43a",
+    "hash": "06a7ab1e9202f62b6408631ab726a054228f0619",
     "id": "b#test",
     "label": "b - test",
     "options": {},
@@ -52,7 +52,7 @@ exports[`info command basic info test case 1`] = `
     "dependents": [
       "a#test",
     ],
-    "hash": "ed53172d402c3e6a65320e54a4e376bab9caa4e4",
+    "hash": "1fad316197c0a2ddeb24dd58ea789786f7e9344b",
     "id": "a#build",
     "label": "a - build",
     "options": {},
@@ -72,7 +72,7 @@ exports[`info command basic info test case 1`] = `
       "b#test",
       "a#build",
     ],
-    "hash": "95ce59ba59bc59830855743a779d2cb50d069c33",
+    "hash": "f0cba92f4a986ccab32a5eb46a7936096224d441",
     "id": "b#build",
     "label": "b - build",
     "options": {},
@@ -96,7 +96,7 @@ exports[`info command scoped info test case 1`] = `
       "b#build",
     ],
     "dependents": [],
-    "hash": "5a9536691284014e65dd9fffd8a60552c0a5d43a",
+    "hash": "06a7ab1e9202f62b6408631ab726a054228f0619",
     "id": "b#test",
     "label": "b - test",
     "options": {},
@@ -115,7 +115,7 @@ exports[`info command scoped info test case 1`] = `
     "dependents": [
       "b#test",
     ],
-    "hash": "95ce59ba59bc59830855743a779d2cb50d069c33",
+    "hash": "f0cba92f4a986ccab32a5eb46a7936096224d441",
     "id": "b#build",
     "label": "b - build",
     "options": {},

packages/e2e-tests/src/basic.test.ts

@@ -24,6 +24,33 @@ describe("basics", () => {
     repo.cleanup();
   });
 
+  it.only("basic with missing script names - logging should not include those targets", () => {
+    const repo = new Monorepo("basics-missing-scripts");
+
+    repo.init();
+    repo.addPackage("a", ["b"]);
+    repo.addPackage("b", [], {
+      build: "node ./build.js",
+      test: "node ./test.js",
+      lint: "node ./lint.js",
+      extra: "node ./extra.js",
+    });
+
+    repo.install();
+
+    const results = repo.run("extra");
+    const output = results.stdout + results.stderr;
+    const jsonOutput = parseNdJson(output);
+
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "b", "extra", "success"))).toBeTruthy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "b", "test", "success"))).toBeFalsy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "build", "success"))).toBeFalsy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "test", "success"))).toBeFalsy();
+    expect(jsonOutput.find((entry) => filterEntry(entry.data, "a", "lint", "success"))).toBeFalsy();
+
+    repo.cleanup();
+  });
+
   it("basic test case - with task args", () => {
     const repo = new Monorepo("basics-with-task-args");
 

packages/e2e-tests/src/mock/monorepo.ts

@@ -47,6 +47,7 @@ export class Monorepo {
           test: `node ${this.yarnPath} lage test --reporter json --log-level silly`,
           lint: `node ${this.yarnPath} lage lint --reporter json --log-level silly`,
           clear: `node ${this.yarnPath} lage cache --clear --reporter json --log-level silly`,
+          extra: `node ${this.yarnPath} lage extra --clear --reporter json --log-level silly`,
         },
         devDependencies: {
           "@lage-run/lage": path.resolve(__dirname, "..", "..", "..", "lage2"),
@@ -56,7 +57,8 @@ export class Monorepo {
         pipeline: {
           build: ['^build'],
           test: ['build'],
-          lint: []
+          lint: [],
+          extra: []
         }
       };`,
       ".gitignore": "node_modules",
@@ -74,6 +76,7 @@ export class Monorepo {
       [`packages/${name}/build.js`]: `console.log('building ${name}');`,
       [`packages/${name}/test.js`]: `console.log('building ${name}');`,
       [`packages/${name}/lint.js`]: `console.log('linting ${name}');`,
+      [`packages/${name}/extra.js`]: `console.log('extra ${name}');`,
       [`packages/${name}/package.json`]: {
         name,
         version: "0.1.0",

packages/e2e-tests/src/transitiveTaskDeps.test.ts

@@ -204,12 +204,6 @@ describe("transitive task deps test", () => {
     // own package transpilation should not be run, since we only want to to consider transitive
     // dependencies with a ^^ dependency.
     expect(indices[getTargetId("a", "transpile")]).toBeUndefined();
-    // despite b depending on c as a package, there is no dependency between the b#transpile and c#transpile
-    // tasks, so they should be runnable in either order.
-    //
-    // In this test we use priority to ensure that b#transpile will always run before
-    // c#transpile if they do not have an explicit task dependency.
-    expect(indices[getTargetId("b", "transpile")]).toBeLessThan(indices[getTargetId("c", "transpile")]);
 
     repo.cleanup();
   });

packages/scheduler/src/SimpleScheduler.ts

@@ -295,7 +295,6 @@ export class SimpleScheduler implements TargetScheduler {
           if (shouldRun) {
             await target.run();
           } else {
-            target.onStart(0);
             target.onSkipped();
           }
         } catch (e) {

packages/scheduler/src/WrappedTarget.ts

@@ -111,15 +111,21 @@ export class WrappedTarget implements TargetRun {
   }
 
   onSkipped(hash?: string | undefined) {
+    if (this.startTime[0] !== 0 && this.startTime[1] !== 0) {
+      this.duration = process.hrtime(this.startTime);
+    }
+
     this.status = "skipped";
-    this.duration = process.hrtime(this.startTime);
-    this.options.logger.info("", {
-      target: this.target,
-      status: "skipped",
-      duration: this.duration,
-      hash,
-      threadId: this.threadId,
-    });
+
+    if (hash) {
+      this.options.logger.info("", {
+        target: this.target,
+        status: "skipped",
+        duration: this.duration,
+        hash,
+        threadId: this.threadId,
+      });
+    }
   }
 
   async getCache() {