Repository: karma-runner/karma (language: javascript)

Issue:
restartOnFileChange option is broken (again)
(See similar issue from 2015: https://github.com/karma-runner/karma/issues/1640)

### Expected behaviour
✅  When `restartOnFileChange: false`, after a test run completes, saving a test file re-runs the test(s)
✅  When `restartOnFileChange: true`, after a test run completes, saving a test file re-runs the test(s)

Here I saved `test.js` three times, and it ran four times (initial run, plus 3 x saves):

![expected](https://user-images.githubusercontent.com/289327/140013338-f69b24cd-84b4-4565-ba67-450b21dbf0eb.png)

### Actual behaviour
✅  When `restartOnFileChange: false`, after a test run completes, saving a test file re-runs the test(s)
🚫  When `restartOnFileChange: true`, after a test run completes, saving a test file does not re-run the test(s)

Here I saved `test.js` four times, and it ran once (initial run only):

![actual](https://user-images.githubusercontent.com/289327/140013383-3b9f4c28-b0ee-4191-ac6b-03a37188b4e4.png)

### Steps to reproduce
I have created a trivial reproduction of the issue here:
https://github.com/scottohara/karma-restartonfilechange-bug



Discussion / hints:
Thanks for the report. I can reproduce it, but not every time. It looks like it is caused by the race condition between the `stop` and `execute` signals sent to the browser. I'll look into it more.

Fix patch:
diff --git a/client/karma.js b/client/karma.js
index 0cc7556e0..64264d1d0 100644
--- a/client/karma.js
+++ b/client/karma.js
@@ -232,20 +232,15 @@ function Karma (updater, socket, iframe, opener, navigator, location, document)
       resultsBuffer = []
     }
 
-    // A test could have incorrectly issued a navigate. Wait one turn
-    // to ensure the error from an incorrect navigate is processed.
-    var config = this.config
-    setTimeout(function () {
-      socket.emit('complete', result || {})
-      if (config.clearContext) {
-        navigateContextTo('about:blank')
-      } else {
-        self.updater.updateTestStatus('complete')
-      }
-      if (returnUrl) {
-        location.href = returnUrl
-      }
-    })
+    socket.emit('complete', result || {})
+    if (this.config.clearContext) {
+      navigateContextTo('about:blank')
+    } else {
+      self.updater.updateTestStatus('complete')
+    }
+    if (returnUrl) {
+      location.href = returnUrl
+    }
   }
 
   this.info = function (info) {
diff --git a/static/karma.js b/static/karma.js
index 50be7e472..b88001881 100644
--- a/static/karma.js
+++ b/static/karma.js
@@ -242,20 +242,15 @@ function Karma (updater, socket, iframe, opener, navigator, location, document)
       resultsBuffer = []
     }
 
-    // A test could have incorrectly issued a navigate. Wait one turn
-    // to ensure the error from an incorrect navigate is processed.
-    var config = this.config
-    setTimeout(function () {
-      socket.emit('complete', result || {})
-      if (config.clearContext) {
-        navigateContextTo('about:blank')
-      } else {
-        self.updater.updateTestStatus('complete')
-      }
-      if (returnUrl) {
-        location.href = returnUrl
-      }
-    })
+    socket.emit('complete', result || {})
+    if (this.config.clearContext) {
+      navigateContextTo('about:blank')
+    } else {
+      self.updater.updateTestStatus('complete')
+    }
+    if (returnUrl) {
+      location.href = returnUrl
+    }
   }
 
   this.info = function (info) {


Test patch:
diff --git a/test/e2e/restart-on-change.feature b/test/e2e/restart-on-change.feature
new file mode 100644
index 000000000..7ccaa8fb8
--- /dev/null
+++ b/test/e2e/restart-on-change.feature
@@ -0,0 +1,28 @@
+Feature: Restart on file change
+  In order to use Karma
+  As a person who wants to write great tests
+  I want Karma to re-run tests whenever file changes.
+
+  Scenario: Re-run tests when file changes
+    Given a configuration with:
+      """
+      files = ['basic/plus.js', 'basic/test.js'];
+      browsers = ['ChromeHeadlessNoSandbox'];
+      plugins = [
+        'karma-jasmine',
+        'karma-chrome-launcher'
+      ];
+      restartOnFileChange = true;
+      singleRun = false;
+      """
+    When I start a server in background
+    And I wait until server output contains:
+      """
+      ..
+      Chrome Headless
+      """
+    When I touch file: "basic/test.js"
+    Then the background stdout matches RegExp:
+      """
+      Executed 2 of 2 SUCCESS[\s\S]+Executed 2 of 2 SUCCESS
+      """
diff --git a/test/e2e/step_definitions/core_steps.js b/test/e2e/step_definitions/core_steps.js
index c9ed84a74..6b2cb31cd 100644
--- a/test/e2e/step_definitions/core_steps.js
+++ b/test/e2e/step_definitions/core_steps.js
@@ -27,7 +27,11 @@ When('I stop a server programmatically', function (callback) {
 })
 
 When('I start a server in background', async function () {
-  await this.runBackgroundProcess(['start', '--log-level', 'debug', this.configFile])
+  await this.runBackgroundProcess(['start', this.configFile])
+})
+
+When('I start a server in background with additional arguments: {string}', async function (args) {
+  await this.runBackgroundProcess(['start', ...args.split(' '), this.configFile])
 })
 
 When('I wait until server output contains:', async function (expectedOutput) {
@@ -50,6 +54,11 @@ When('I {command} Karma', async function (command) {
   await this.runForegroundProcess(`${command} ${this.configFile}`)
 })
 
+When('I touch file: {string}', async function (file) {
+  const now = new Date()
+  await fs.promises.utimes(path.join(this.workDir, file), now, now)
+})
+
 When('I {command} Karma with additional arguments: {string}', async function (command, args) {
   await this.runForegroundProcess(`${command} ${this.configFile} ${args}`)
 })
@@ -149,3 +158,27 @@ Then(/^the file at ([a-zA-Z0-9/\\_.]+) contains:$/, function (filePath, expected
     throw new Error('Expected output to match the following:\n  ' + expectedOutput + '\nGot:\n  ' + data)
   }
 })
+
+Then(/^the background (stdout|stderr) (is exactly|contains|matches RegExp):$/, async function (outputType, comparison, expectedOutput) {
+  const message = comparison === 'is exactly' ? 'Expected output to be exactly as above, but got:'
+    : comparison === 'contains' ? 'Expected output to contain the above text, but got:'
+      : 'Expected output to match the above RegExp, but got:'
+
+  await waitForCondition(
+    () => {
+      const actualOutput = this.backgroundProcess[outputType].trim()
+      expectedOutput = expectedOutput.trim()
+
+      switch (comparison) {
+        case 'is exactly':
+          return actualOutput === expectedOutput
+        case 'contains':
+          return actualOutput.includes(expectedOutput)
+        case 'matches RegExp':
+          return new RegExp(expectedOutput).test(actualOutput)
+      }
+    },
+    5000,
+    () => new Error(`${message}\n\n${this.backgroundProcess[outputType]}`)
+  )
+})