fix(docstring.feature): extend DocString.feature tests to cover failing scenario

[ralphsaunders]

Test case for reported issue #415

DocString.feature will fail when ES6 template literals are present in the DocString.

Open to suggestions/support on how this can be fixed.

[lgandecki]

Thanks for this :) It's always helpful to start with a failing test.
If no one else picks it up I will try. Should be doable, although it might be a bit tricky.

[badeball]

I believe the following patch will suffice. Using single- or double-quotes is per js-string-escape' specs.

diff --git a/lib/featuresLoader.js b/lib/featuresLoader.js
index 266db34..a380f23 100644
--- a/lib/featuresLoader.js
+++ b/lib/featuresLoader.js
@@ -61,9 +61,9 @@ const createCucumber = (
             .find(fileSteps => fileSteps[filePath])
             [filePath].join("\n")}
             
-        createTestsFromFeature('${path.basename(filePath)}', \`${jsStringEscape(
+        createTestsFromFeature('${path.basename(filePath)}', "${jsStringEscape(
         spec
-      )}\`);
+      )}");
         })
         `

[lgandecki]

nice! @ralphsaunders are you able to test that?

[ralphsaunders]

Unfortunately @lgandecki @badeball the suggested change is failing the test case. I did notice the create cucumber fn returns an ES6 template literal itself but unsure if that's really the problem.

I read the js-string-escape implementation and saw it has ES5 support. ES6 introduced template literals so I suppose the escaping rules may need to be extended to catch the two character sequence ${ (relevant spec).

js-string-escape does have a relevant issue raised. Perhaps we could find an alternative package if js-string-escape is as dead as it first appears?

[badeball]

My bad, I had just left out the most important case. Try this and if it works, then there's no need to wait for their maintainer to do anything.

diff --git a/lib/featuresLoader.js b/lib/featuresLoader.js
index 266db34..a380f23 100644
--- a/lib/featuresLoader.js
+++ b/lib/featuresLoader.js
@@ -61,9 +61,9 @@ const createCucumber = (
             .find(fileSteps => fileSteps[filePath])
             [filePath].join("\n")}
             
-        createTestsFromFeature('${path.basename(filePath)}', \`${jsStringEscape(
+        createTestsFromFeature('${path.basename(filePath)}', "${jsStringEscape(
         spec
-      )}\`);
+      )}");
         })
         `
     )
diff --git a/lib/loader.js b/lib/loader.js
index 6edabd1..5945928 100644
--- a/lib/loader.js
+++ b/lib/loader.js
@@ -15,7 +15,7 @@ const createCucumber = (filePath, cucumberJson, spec, toRequire, name) =>
   window.cucumberJson = ${JSON.stringify(cucumberJson)};
   describe(\`${name}\`, function() {
     ${toRequire.join("\n")}
-    createTestsFromFeature('${filePath}', \`${jsStringEscape(spec)}\`);
+    createTestsFromFeature('${filePath}', "${jsStringEscape(spec)}");
   });
   `;

[badeball]

Btw, I believe JSON is practically a subset of JS, except for two peculiar unicode characters, so it should be pretty straight forward to remove that dependency entirely.

Edit: To extend on this: I mean that you can just use JSON.stringify(myString) to obtain nearly the same thing.

[ralphsaunders]

Ah I hadn't read your first diff properly @badeball and so missed there was another way specs were loaded in. Adjusting both as the new test case passing! :)

[lgandecki]

great, thank you guys :)

[lgandecki]

🎉 This PR is included in version 2.5.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀