From ef7486d3f5256e3cd752e853df9950ac03d469e4 Mon Sep 17 00:00:00 2001
From: Lukas Taegert-Atkinson <lukastaegert@users.noreply.github.com>
Date: Thu, 4 Jul 2019 16:16:36 +0200
Subject: [PATCH] Make tests run on Node 12, fix watcher cleanup issue (#2982)

* Make tests run on Node 12, fix watcher cleanup issue

* Just do not run leak test on Node 12
---
 .circleci/config.yml                          | 19 +++++++++++++++++++
 package.json                                  |  1 +
 src/watch/index.ts                            | 14 +++++++-------
 .../catch-dynamic-import-failure/_config.js   |  3 ++-
 .../dynamic-import-expression/_config.js      |  5 ++++-
 .../dynamic-import-rewriting/_config.js       |  5 ++++-
 6 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/.circleci/config.yml b/.circleci/config.yml
index 70ff0652971..bbd2f6a3d1c 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -9,6 +9,15 @@ unit_tests: &unit_tests
         name: Run unit tests.
         command: npm run ci:test
 
+unit_tests_12: &unit_tests_12
+  steps:
+    - checkout
+    - restore_cache:
+        key: dependency-cache-{{ checksum "package-lock.json" }}
+    - run:
+        name: Run unit tests.
+        command: npm run ci:test_12
+
 jobs:
   analysis:
     docker:
@@ -42,6 +51,10 @@ jobs:
     docker:
       - image: rollupcabal/circleci-node-v10:latest
     <<: *unit_tests
+  node-v12-latest:
+    docker:
+      - image: rollupcabal/circleci-node-v12:latest
+    <<: *unit_tests_12
 
 workflows:
   version: 2
@@ -69,3 +82,9 @@ workflows:
           filters:
             tags:
               only: /.*/
+      - node-v12-latest:
+          requires:
+            - analysis
+          filters:
+            tags:
+              only: /.*/
diff --git a/package.json b/package.json
index 5019e432ea1..070b411537a 100644
--- a/package.json
+++ b/package.json
@@ -16,6 +16,7 @@
     "ci:coverage": "npm run test:coverage",
     "ci:lint": "npm run lint:nofix && npm run security",
     "ci:test": "npm run build:test && npm run build:bootstrap && npm run test:all",
+    "ci:test_12": "npm run build:test && npm run build:bootstrap && npm run test:only && npm run test:typescript",
     "lint": "npm run lint:ts -- --fix && npm run lint:js -- --fix && npm run lint:markdown",
     "lint:nofix": "npm run lint:ts && npm run lint:js && npm run lint:markdown",
     "lint:ts": "tslint --project .",
diff --git a/src/watch/index.ts b/src/watch/index.ts
index 5086697cf0b..f2757497549 100644
--- a/src/watch/index.ts
+++ b/src/watch/index.ts
@@ -211,26 +211,26 @@ export class Task {
 		return rollup(options)
 			.then(result => {
 				if (this.closed) return undefined as any;
-
+				const previouslyWatched = this.watched;
 				const watched = (this.watched = new Set());
 
 				this.cache = result.cache;
 				this.watchFiles = result.watchFiles;
-				(this.cache.modules as ModuleJSON[]).forEach(module => {
+				for (const module of this.cache.modules as ModuleJSON[]) {
 					if (module.transformDependencies) {
 						module.transformDependencies.forEach(depId => {
 							watched.add(depId);
 							this.watchFile(depId, true);
 						});
 					}
-				});
-				this.watchFiles.forEach(id => {
+				}
+				for (const id of this.watchFiles) {
 					watched.add(id);
 					this.watchFile(id);
-				});
-				this.watched.forEach(id => {
+				}
+				for (const id of previouslyWatched) {
 					if (!watched.has(id)) deleteTask(id, this, this.chokidarOptionsHash);
-				});
+				}
 
 				return Promise.all(this.outputs.map(output => result.write(output))).then(() => result);
 			})
diff --git a/test/function/samples/catch-dynamic-import-failure/_config.js b/test/function/samples/catch-dynamic-import-failure/_config.js
index 475d2078603..98030f30575 100644
--- a/test/function/samples/catch-dynamic-import-failure/_config.js
+++ b/test/function/samples/catch-dynamic-import-failure/_config.js
@@ -10,7 +10,8 @@ module.exports = {
 		return exports.then(result => {
 			assert.strictEqual(result[0].message, 'exists-named');
 			assert.strictEqual(result[1].message, 'exists-default');
-			assert.strictEqual(result[2].message, "Cannot find module 'does-not-exist'");
+			const expectedError = "Cannot find module 'does-not-exist'";
+			assert.strictEqual(result[2].message.slice(0, expectedError.length), expectedError);
 		});
 	}
 };
diff --git a/test/function/samples/dynamic-import-expression/_config.js b/test/function/samples/dynamic-import-expression/_config.js
index 4844bfc3fd4..e759d62c9af 100644
--- a/test/function/samples/dynamic-import-expression/_config.js
+++ b/test/function/samples/dynamic-import-expression/_config.js
@@ -24,6 +24,9 @@ module.exports = {
 		]
 	},
 	exports(exports) {
-		return exports.catch(err => assert.strictEqual(err.message, "Cannot find module 'x/y'"));
+		const expectedError = "Cannot find module 'x/y'";
+		return exports.catch(err =>
+			assert.strictEqual(err.message.slice(0, expectedError.length), expectedError)
+		);
 	}
 };
diff --git a/test/function/samples/dynamic-import-rewriting/_config.js b/test/function/samples/dynamic-import-rewriting/_config.js
index 5d2dce5b68c..a34764a8406 100644
--- a/test/function/samples/dynamic-import-rewriting/_config.js
+++ b/test/function/samples/dynamic-import-rewriting/_config.js
@@ -13,6 +13,9 @@ module.exports = {
 		]
 	},
 	exports(exports) {
-		return exports.promise.catch(err => assert.equal(err.message, "Cannot find module 'asdf'"));
+		const expectedError = "Cannot find module 'asdf'";
+		return exports.promise.catch(err =>
+			assert.strictEqual(err.message.slice(0, expectedError.length), expectedError)
+		);
 	}
 };