Merge pull request #216 from jcundill/fixBeforeHooks

fix: Stop cucumber Before hook clobbering mocha before.
badeball · Aug 24, 2019 · 1f8bcfb · 1f8bcfb
2 parents c321a17 + 3b9ccec
commit 1f8bcfb
Show file tree

Hide file tree

Showing 6 changed files with 134 additions and 92 deletions.
diff --git a/README.md b/README.md
@@ -102,7 +102,7 @@ This is a good place to put global before/beforeEach/after/afterEach hooks.
 The problem with the legacy structure is that everything is global. This is problematic for multiple reasons.
 - It makes it harder to create .feature files that read nicely - you have to make sure you are not stepping on toes of already existing step definitions. You should be able to write your tests without worrying about reusability, complex regexp matches, or anything like that. Just write a story. Explain what you want to see without getting into the details. Reuse in the .js files, not in something you should consider an always up-to-date, human-readable documentation.
 - The startup times get much worse - because we have to analyze all the different step definitions so we can match the .feature files to the test.
-- Hooks are problematic. If you put before() in a step definition file, you might think that it will run only for the .feature file related to that step definition. You try the feature you work on, everything seems fine and you push the code. Here comes a surprise - it will run for ALL .feature files in your whole project. Very unintuitive. And good luck debugging problems caused by that! This problem was not unique to this plugin, bo to the way cucumberjs operates. 
+- Hooks are problematic. If you put before() in a step definition file, you might think that it will run only for the .feature file related to that step definition. You try the feature you work on, everything seems fine and you push the code. Here comes a surprise - it will run for ALL .feature files in your whole project. Very unintuitive. And good luck debugging problems caused by that! This problem was not unique to this plugin, but to the way cucumberjs operates. 
  Let's look how this differs with the proposed structure. Assuming you want to have a hook before ./Google.feature file, just create a ./Google/before.js and put the hook there. This should take care of long requested feature - [https://github.com/TheBrainFamily/cypress-cucumber-preprocessor/issues/25](#25)
 
 If you have a few tests the "oldschool" style is completely fine. But for a large enterprise-grade application, with hundreds or sometimes thousands of .feature files, the fact that everything is global becomes a maintainability nightmare. 
@@ -147,7 +147,7 @@ Then(`I see {string} in the title`, (title) => {
 ```
 
 
-#### Given/When/Then functions
+### Given/When/Then functions
 
 Since Given/When/Then are on global scope please use
 ```javascript
@@ -157,6 +157,46 @@ to make IDE/linter happy
 
 or import them directly as shown in the above examples
 
+### Cucumber Before and After hooks
+
+The cypress-cucumber-preprocessor supports both the mocha `before/beforeEach/after/afterEach` hooks and cucumber `Before` and `After` hooks.
+
+The cucumber hooks implementation fully supports tagging as described in [the cucumber js documentation](https://github.com/cucumber/cucumber-js/blob/master/docs/support_files/hooks.md). So they can be conditionally selected based on the tags applied to the Scenario. This is not possible with mocha hooks.
+
+Cucumber Before hooks run after all mocha before and beforeEach hooks have completed and the cucumber After hooks run before all the mocha afterEach and after hooks.
+
+#### Using cucumber hooks
+
+```javascript
+const {
+  Before,
+  After,
+  Given,
+  Then
+} = require("cypress-cucumber-preprocessor/steps");
+
+// this will get called before each scenario
+Before(() => {
+  beforeCounter += 1;
+  beforeWithTagCounter = 0;
+});
+
+// this will only get called before scenarios tagged with @foo
+Before({ tags: "@foo" }, () => {
+  beforeWithTagCounter += 1;
+});
+
+Given("My Step Definition", () => {
+  // ...test code here
+})
+
+```
+
+Note: to avoid confusion with the similarly named mocha before and after hooks, the cucumber hooks are not exported onto global scope. So they need explicitly importing as shown above.
+
+
+
+
 ### Running
 
 Run your cypress the way you would usually do, for example:

diff --git a/lib/createTestFromScenario.js b/lib/createTestFromScenario.js
@@ -63,85 +63,82 @@ const createTestFromScenarios = (
   backgroundSection,
   testState
 ) => {
-  // eslint-disable-next-line func-names
-  describe(testState.feature.name, function() {
-    before(() => {
-      cy.then(() => testState.onStartTest());
-    });
+  // eslint-disable-next-line func-names, prefer-arrow-callback
+  before(function() {
+    cy.then(() => testState.onStartTest());
+  });
 
-    // ctx is cleared between each 'it'
-    // eslint-disable-next-line func-names, prefer-arrow-callback
-    beforeEach(function() {
-      window.testState = testState;
+  // ctx is cleared between each 'it'
+  // eslint-disable-next-line func-names, prefer-arrow-callback
+  beforeEach(function() {
+    window.testState = testState;
 
-      const failHandler = err => {
-        Cypress.off("fail", failHandler);
-        testState.onFail(err);
-        throw err;
-      };
+    const failHandler = err => {
+      Cypress.off("fail", failHandler);
+      testState.onFail(err);
+      throw err;
+    };
 
-      Cypress.on("fail", failHandler);
-    });
+    Cypress.on("fail", failHandler);
+  });
 
-    scenariosToRun.forEach(section => {
-      if (section.examples) {
-        section.examples.forEach(example => {
-          const exampleValues = [];
-          const exampleLocations = [];
-
-          example.tableBody.forEach((row, rowIndex) => {
-            exampleLocations[rowIndex] = row.location;
-            example.tableHeader.cells.forEach((header, headerIndex) => {
-              exampleValues[rowIndex] = Object.assign(
-                {},
-                exampleValues[rowIndex],
-                {
-                  [header.value]: row.cells[headerIndex].value
-                }
-              );
-            });
+  scenariosToRun.forEach(section => {
+    if (section.examples) {
+      section.examples.forEach(example => {
+        const exampleValues = [];
+        const exampleLocations = [];
+
+        example.tableBody.forEach((row, rowIndex) => {
+          exampleLocations[rowIndex] = row.location;
+          example.tableHeader.cells.forEach((header, headerIndex) => {
+            exampleValues[rowIndex] = Object.assign(
+              {},
+              exampleValues[rowIndex],
+              {
+                [header.value]: row.cells[headerIndex].value
+              }
+            );
           });
+        });
 
-          exampleValues.forEach((rowData, index) => {
-            // eslint-disable-next-line prefer-arrow-callback
-            const scenarioName = replaceParameterTags(rowData, section.name);
-            const uniqueScenarioName = `${scenarioName} (example #${index +
-              1})`;
-            const exampleSteps = section.steps.map(step => {
-              const newStep = Object.assign({}, step);
-              newStep.text = replaceParameterTags(rowData, newStep.text);
-              return newStep;
-            });
-
-            const stepsToRun = backgroundSection
-              ? backgroundSection.steps.concat(exampleSteps)
-              : exampleSteps;
-
-            const scenarioExample = Object.assign({}, section, {
-              name: uniqueScenarioName,
-              example: exampleLocations[index]
-            });
-
-            runTest.call(this, scenarioExample, stepsToRun, rowData);
+        exampleValues.forEach((rowData, index) => {
+          // eslint-disable-next-line prefer-arrow-callback
+          const scenarioName = replaceParameterTags(rowData, section.name);
+          const uniqueScenarioName = `${scenarioName} (example #${index + 1})`;
+          const exampleSteps = section.steps.map(step => {
+            const newStep = Object.assign({}, step);
+            newStep.text = replaceParameterTags(rowData, newStep.text);
+            return newStep;
           });
-        });
-      } else {
-        const stepsToRun = backgroundSection
-          ? backgroundSection.steps.concat(section.steps)
-          : section.steps;
 
-        runTest.call(this, section, stepsToRun);
-      }
-    });
+          const stepsToRun = backgroundSection
+            ? backgroundSection.steps.concat(exampleSteps)
+            : exampleSteps;
+
+          const scenarioExample = Object.assign({}, section, {
+            name: uniqueScenarioName,
+            example: exampleLocations[index]
+          });
 
-    // eslint-disable-next-line func-names, prefer-arrow-callback
-    after(function() {
-      cy.then(() => testState.onFinishTest()).then(() => {
-        if (window.cucumberJson && window.cucumberJson.generate) {
-          const json = generateCucumberJson(testState);
-          writeCucumberJsonFile(json);
-        }
+          runTest.call(this, scenarioExample, stepsToRun, rowData);
+        });
       });
+    } else {
+      const stepsToRun = backgroundSection
+        ? backgroundSection.steps.concat(section.steps)
+        : section.steps;
+
+      runTest.call(this, section, stepsToRun);
+    }
+  });
+
+  // eslint-disable-next-line func-names, prefer-arrow-callback
+  after(function() {
+    cy.then(() => testState.onFinishTest()).then(() => {
+      if (window.cucumberJson && window.cucumberJson.generate) {
+        const json = generateCucumberJson(testState);
+        writeCucumberJsonFile(json);
+      }
     });
   });
 };

diff --git a/lib/loader.js b/lib/loader.js
@@ -2,31 +2,31 @@
 const log = require("debug")("cypress:cucumber");
 const path = require("path");
 const cosmiconfig = require("cosmiconfig");
+const { Parser } = require("gherkin");
 const { getStepDefinitionsPaths } = require("./getStepDefinitionsPaths");
 
 // This is the template for the file that we will send back to cypress instead of the text of a
 // feature file
-const createCucumber = (filePath, cucumberJson, spec, toRequire) =>
+const createCucumber = (filePath, cucumberJson, spec, toRequire, name) =>
   `
-  const {resolveAndRunStepDefinition, defineParameterType, given, when, then, and, but, before, after, defineStep} = require('cypress-cucumber-preprocessor/lib/resolveStepDefinition');
+  const {resolveAndRunStepDefinition, defineParameterType, given, when, then, and, but, Before, After, defineStep} = require('cypress-cucumber-preprocessor/lib/resolveStepDefinition');
   const Given = window.Given = window.given = given;
   const When = window.When = window.when = when;
   const Then = window.Then = window.then = then;
   const And = window.And = window.and = and;
   const But = window.But = window.but = but;
-  const Before = window.Before = window.before = before;
-  const After = window.After = window.after = after;
   window.defineParameterType = defineParameterType;
   window.defineStep = defineStep;
-  const { createTestsFromFeature } = require('cypress-cucumber-preprocessor/lib/createTestsFromFeature');
-  ${eval(toRequire).join("\n")}
-  const spec = \`${spec}\`;
-  const filePath = '${filePath}';
   window.cucumberJson = ${JSON.stringify(cucumberJson)};
-     
-  createTestsFromFeature(filePath, spec);
+  const { createTestsFromFeature } = require('cypress-cucumber-preprocessor/lib/createTestsFromFeature');
+  
+  describe(\`${name}\`, function() {
+    ${eval(toRequire).join("\n")}
+    createTestsFromFeature('${filePath}', \`${spec}\`);
+  });
   `;
 
+// eslint-disable-next-line func-names
 module.exports = function(spec, filePath = this.resourcePath) {
   const explorer = cosmiconfig("cypress-cucumber-preprocessor", { sync: true });
   const loaded = explorer.load();
@@ -40,10 +40,12 @@ module.exports = function(spec, filePath = this.resourcePath) {
   const stepDefinitionsToRequire = getStepDefinitionsPaths(filePath).map(
     sdPath => `require('${sdPath}')`
   );
+  const { name } = new Parser().parse(spec.toString()).feature;
   return createCucumber(
     path.basename(filePath),
     cucumberJson,
     spec,
-    stepDefinitionsToRequire
+    stepDefinitionsToRequire,
+    name
   );
 };
diff --git a/lib/resolveStepDefinition.js b/lib/resolveStepDefinition.js
@@ -200,11 +200,11 @@ module.exports = {
   but: (expression, implementation) => {
     stepDefinitionRegistry.runtime(expression, implementation);
   },
-  before: (...args) => {
+  Before: (...args) => {
     const { tags, implementation } = parseHookArgs(args);
     beforeHookRegistry.runtime(tags, implementation);
   },
-  after: (...args) => {
+  After: (...args) => {
     const { tags, implementation } = parseHookArgs(args);
     afterHookRegistry.runtime(tags, implementation);
   },

diff --git a/lib/testHelpers/setupTestFramework.js b/lib/testHelpers/setupTestFramework.js
@@ -1,6 +1,9 @@
 global.jestExpect = global.expect;
 global.expect = require("chai").expect;
 
+global.before = jest.fn();
+global.after = jest.fn();
+
 window.Cypress = {
   env: jest.fn(),
   on: jest.fn(),
@@ -17,8 +20,8 @@ const {
   given,
   and,
   but,
-  before,
-  after
+  Before,
+  After
 } = require("../resolveStepDefinition");
 
 const mockedPromise = jest.fn().mockImplementation(() => Promise.resolve(true));
@@ -29,8 +32,8 @@ window.then = then;
 window.given = given;
 window.and = and;
 window.but = but;
-window.before = before;
-window.after = after;
+window.Before = Before;
+window.After = After;
 window.defineStep = defineStep;
 window.cy = {
   log: jest.fn(),

diff --git a/steps.js b/steps.js
@@ -7,8 +7,8 @@ const {
   then,
   and,
   but,
-  before,
-  after,
+  Before,
+  After,
   defineParameterType,
   defineStep
 } = require("./lib/resolveStepDefinition");
@@ -24,8 +24,8 @@ module.exports = {
   Then: then,
   And: and,
   But: but,
-  Before: before,
-  After: after,
+  Before,
+  After,
   defineParameterType,
   defineStep
 };