Merge pull request #10 from savetheclocktower/ci-only-tests

test: add CI-only tests

Author: savetheclocktower

.github/workflows/CI.yml

@@ -26,9 +26,51 @@ jobs:
       - uses: actions/setup-node@v3
         with:
           node-version: ${{ matrix.node_version }}
+      - uses: volta-cli/action@v1.7.0
+      - name: Install multiple Node versions
+        if: runner.os != 'Windows'
+        run: |
+          volta install node@10
+          echo "NODE_10=$(volta which node)" >> $GITHUB_ENV
+          echo "NODE_10_VERSION=$(node --version)" >> $GITHUB_ENV
+          volta install node@latest
+          echo "NODE_LATEST=$(volta which node)" >> $GITHUB_ENV
+          echo "NODE_LATEST_VERSION=$(node --version)" >> $GITHUB_ENV
+          volta install node@16
+          echo "NODE_DEFAULT=$(volta which node)" >> $GITHUB_ENV
+          echo "NODE_DEFAULT_VERSION=$(node --version)" >> $GITHUB_ENV
+      - name: Install multiple Node versions (Windows)
+        if: runner.os == 'Windows'
+        run: |
+          volta install node@10
+          echo "NODE_10=$(volta which node)" >> $env:GITHUB_ENV
+          echo "NODE_10_VERSION=$(node --version)" >> $env:GITHUB_ENV
+          volta install node@latest
+          echo "NODE_LATEST=$(volta which node)" >> $env:GITHUB_ENV
+          echo "NODE_LATEST_VERSION=$(node --version)" >> $env:GITHUB_ENV
+          volta install node@16
+          echo "NODE_DEFAULT=$(volta which node)" >> $env:GITHUB_ENV
+          echo "NODE_DEFAULT_VERSION=$(node --version)" >> $env:GITHUB_ENV
+      - name: Setup dummy ESLint projects
+        run: |
+          mkdir ~/with-eslint-6 && cd ~/with-eslint-6
+          npm init --yes
+          npm install eslint@6
+          cd ..
+
+          mkdir ~/with-eslint-7 && cd ~/with-eslint-7
+          npm init --yes
+          npm install eslint@7
+          cd ..
+
+          mkdir ~/with-eslint-latest && cd ~/with-eslint-latest
+          npm init --yes
+          npm install eslint@latest
+          cd ..
       - name: Install dependencies
         run: |
           apm install
+          apm install linter-eslint
           # ./node_modules/.bin/atom-package-deps .
 
       - name: Run tests 👩🏾‍💻

lib/job-manager.js

@@ -57,7 +57,11 @@ class JobManager {
 
     let nodeBin = Config.get('nodeBin');
     console.debug('JobManager creating worker at:', nodeBin);
-    this.killWorker();
+
+    // TODO: Figure out if this can even happen anymore.
+    if (this.worker) {
+      this.killWorker(this.worker);
+    }
 
     // We choose to do a sync test here because this method is much easier to
     // reason about without an `await` keyword introducing side effects.
@@ -99,32 +103,36 @@ class JobManager {
       this.worker.stderr
         .pipe(ndjson.parse())
         .on('data', this.receiveError.bind(this));
-
-      this.worker.on('close', () => {
-        if (this.worker.killed === false) {
-          this.createWorker();
-        }
-      });
     });
 
+    let nullWorkerPromise = () => this._workerPromise = null;
+
     this._workerPromise = promise;
     this._workerPromise
-      .then(() => this._workerPromise = null)
-      .catch(() => this._workerPromise = null);
+      .then(nullWorkerPromise)
+      .catch(nullWorkerPromise);
 
     return promise;
   }
 
-  suspend () {
-    console.warn('Suspending worker');
-    this.killWorker();
-    this.worker = null;
+  async suspend () {
+    console.debug('Suspending worker');
+    // To prevent async chaos, we should refrain from killing a worker that
+    // we're in the process of creating.
+    let promise = this._workerPromise || Promise.resolve();
+    this._killingWorkerPromise = promise.then(() => {
+      let worker = this.worker;
+      this.worker = null;
+      this.killWorker(worker);
+      this._killingWorkerPromise = null;
+    });
+    return this._killingWorkerPromise;
   }
 
-  killWorker () {
-    if (!this.worker || this.worker.exitCode) { return; }
-    this.worker.removeAllListeners();
-    this.worker.kill();
+  killWorker (worker) {
+    if (!worker || worker.exitCode) { return; }
+    worker.removeAllListeners();
+    worker.kill();
   }
 
   ensureWorker () {
@@ -170,8 +178,12 @@ class JobManager {
   }
 
   async send (bundle) {
+    if (this._killingWorkerPromise) {
+      console.debug('Waiting for worker to be killed');
+      await this._killingWorkerPromise;
+    }
     if (!this.worker) {
-      console.warn('Creating worker');
+      console.debug('Creating worker');
       await this.createWorker();
     }
 

lib/main.js

@@ -55,7 +55,7 @@ export default {
   },
 
   isLegacyPackagePresent () {
-    return ('linter-eslint' in atom.packages.activePackages);
+    return atom.packages.isPackageActive('linter-eslint');
   },
 
   async activate () {
@@ -132,11 +132,11 @@ export default {
 
           if (config.nodeBin !== prevConfig.nodeBin) {
             this.notified.invalidNodeBin = false;
+            this.jobManager.suspend();
             NodePathTester
               .test(config.nodeBin)
               .then((version) => {
-                console.log(`Switched Node to version:`, version);
-                this.jobManager.suspend();
+                console.info(`Switched Node to version:`, version);
               })
               .catch(() => {
                 this.sleep();
@@ -249,8 +249,8 @@ export default {
   // Show a bunch of stuff to the user that can help them figure out why the
   // package isn't behaving the way they think it ought to, or else give them
   // something to copy and paste into a new issue.
-  async debugJob () {
-    const textEditor = atom.workspace.getActiveTextEditor();
+  async debugJob (editor = null) {
+    const textEditor = editor || atom.workspace.getActiveTextEditor();
     let filePath = 'unknown', editorScopes = ['unknown'];
     if (atom.workspace.isTextEditor(textEditor)) {
       filePath = textEditor.getPath();
@@ -328,7 +328,7 @@ export default {
       }
     } catch (error) {
       atom.notifications.addError(`${error}`, { dismissable: true });
-      return;
+      return {};
     }
 
     let whichPackageWillLint;
@@ -339,6 +339,7 @@ export default {
     } else {
       whichPackageWillLint = 'linter-eslint-node';
     }
+    debug.whichPackageWillLint = whichPackageWillLint;
 
     let debugMessage = [
       `Atom version: ${debug.atomVersion}`,

lib/worker.js

@@ -16,7 +16,6 @@ const MINIMUM_ESLINT_VERSION = '7.0.0';
 const PATHS_CACHE = new Map();
 const ESLINT_CACHE = new Map();
 
-// TODO: There must be some cases where this logic is too naive, right?
 function descendsFrom (filePath, projectPath) {
   if (typeof filePath !== 'string') { return false; }
   return filePath.startsWith(projectPath.endsWith(Path.sep) ? projectPath : `${projectPath}${Path.sep}`);
@@ -414,7 +413,7 @@ async function processMessage (bundle) {
       return;
     }
     error.key = key;
-    error.error = 'Unknown error';
+    error.error = error.message || 'Unknown error';
     emitError(
       JSON.stringify(error, Object.getOwnPropertyNames(error))
     );

spec/.eslintrc.js

@@ -2,5 +2,8 @@ module.exports = {
   env: {
     jasmine: true,
     atomtest: true
+  },
+  globals: {
+    pass: 'readonly'
   }
 };

spec/fixtures/ci/package-interaction/.eslintrc

@@ -0,0 +1,6 @@
+{
+  root: true,
+  rules: {
+    "no-unused-vars": "error"
+  }
+}

spec/fixtures/ci/package-interaction/index.js

@@ -0,0 +1,4 @@
+
+function unusedFunction () {
+  // no-op
+}

spec/helpers.js

@@ -38,12 +38,31 @@ export async function openAndSetProjectDir (fileName, projectDir) {
   let editor = await atom.workspace.open(fileName);
   atom.project.setPaths([projectDir]);
   await race(
-    atom.project.watcherPromisesByPath[projectDir],
+    atom.project.getWatcherPromise(projectDir),
     wait(1000)
   );
   return editor;
 }
 
+export function getNotification (expectedMessage = null) {
+  let promise = new Promise((resolve, reject) => {
+    let notificationSub;
+    let newNotification = notification => {
+      if (expectedMessage && notification.getMessage()) {
+        return;
+      }
+      if (notificationSub !== undefined) {
+        notificationSub.dispose();
+        resolve(notification);
+      } else {
+        reject();
+      }
+    };
+    notificationSub = atom.notifications.onDidAddNotification(newNotification);
+  });
+  return race(promise, wait(3000));
+}
+
 // Grab this before it gets wrapped.
 const _setTimeout = window.setTimeout;
 export function wait (ms) {