Skip to content

Commit

Permalink
fix(main): errors in rendering escaped characters
Browse files Browse the repository at this point in the history
  • Loading branch information
dnotes committed Nov 9, 2024
1 parent 8dd8029 commit 2bb4509
Show file tree
Hide file tree
Showing 7 changed files with 212 additions and 75 deletions.
8 changes: 8 additions & 0 deletions .changeset/silly-suits-attend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"quickpickle": patch
---

Fixed several errors in rendering, including:

- \`Backticks` and ${variables} were not escaped properly in Scenario Outlines
- Backslashes were not escaped properly in other strings
28 changes: 28 additions & 0 deletions packages/main/gherkin-example/example.feature
Original file line number Diff line number Diff line change
Expand Up @@ -124,3 +124,31 @@ Feature: QuickPickle's Comprehensive Gherkin Syntax Example
Given an explodedTags config of [[ '@1a','@1b' ], [ '@2a','@2b' ]]
When this Scenario is run
Then it should be split into 4 tests

Scenario Outline: Search Ordering: <searchPhrase> (<religion>) <context>
When I search for "<searchPhrase>" and get results from 500 books
Then I should see search results with these metrics:
| Book Importance | Match Quality | score |
| primary | exact | 3+5=8 |
And next I should see search results with these metrics:
| secondary | exact | 2+5=7 |
Examples:
| searchPhrase | religion | context |
| ablution | Bahá’í | teachings on prayer |
| achieving enlightenment | Buddhist | purpose of existence |
| achieving immortality | Taoist | purpose of existence |
| acts of kindness | Jewish | teachings on charity |

Scenario Outline: `someone` is ${sneaky} \${with} \\${backslashes} \\\${and} \$\{other} \`things\\` 'like' \'quotes\\'
Given a "string with \"quotes\""
And `someone` is ${sneaky} \${with} \\${backslashes} \\\${and} \$\{other} \`things\\` 'like' \'quotes\\'
Examples:
| 1 | 2 | 3 |
| \'a\' | \`b\` | \"c\" |
| ${a} | \${b} | \\${c} |
| `a` | \`b\` | \\`c\\` |

Scenario: `someone` is ${sneaky} \${with} \\${backslashes} \\\${and} \$\{other} \`things\\` 'like' \'quotes\\'
Given a "string with \"quotes\""
And `someone` is ${sneaky} \${with} \\${backslashes} \\\${and} \$\{other} \`things\\` 'like' \'quotes\\'

145 changes: 91 additions & 54 deletions packages/main/gherkin-example/example.feature.js

Large diffs are not rendered by default.

49 changes: 31 additions & 18 deletions packages/main/src/render.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,6 @@ function renderScenario(child:FeatureChild, config:QuickPickleConfig, tags:strin

let tagTextForVitest = tags.length ? ` (${tags.join(' ')})` : ''

let steps = child.scenario?.steps.map((step,idx) => {
let text = step.text.replace(/`/g, '\\`')
return text
}) || []

// For Scenario Outlines with examples
if (child.scenario!.examples?.[0]?.tableHeader && child.scenario!.examples?.[0]?.tableBody) {

Expand All @@ -169,13 +164,15 @@ function renderScenario(child:FeatureChild, config:QuickPickleConfig, tags:strin
}

let describe = q(replaceParamNames(child.scenario?.name ?? ''))
let name = replaceParamNames(child.scenario?.name ?? '', true).replace(/`/g, '\`')
let scenarioNameWithReplacements = tl(replaceParamNames(child.scenario?.name ?? '', true))

let examples = steps.map((text,idx) => {
let examples = child.scenario?.steps.map(({text},idx) => {
text = replaceParamNames(text,true)
return text
})

let renderedSteps = renderSteps(child.scenario!.steps.map(s => ({...s, text: replaceParamNames(s.text, true)})), config, sp + ' ', isExploded ? `${explodedIdx+1}` : '')

return `
${sp}test${attrs}.for([
${sp} ${paramValues?.map(line => {
Expand All @@ -184,12 +181,8 @@ ${sp} ${paramValues?.map(line => {
${sp}])(
${sp} '${q(child.scenario?.keyword || '')}: ${describe}${tagTextForVitest}',
${sp} async ({ ${origParamNames.map((p,i) => '_'+i)?.join(', ')} }, context) => {
${sp} let state = await ${initFn}(context, \`${name}\`, ['${tags.join("', '") || ''}'], [${examples?.map(s => '`'+s+'`').join(',')}]);
${child.scenario?.steps.map((step,idx) => {
let text = replaceParamNames(step.text,true).replace(/`/g, '\\`')
return `${sp} await gherkinStep(\`${text}\`, state, ${step.location.line}, ${idx+1}${isExploded ? `, ${explodedIdx + 1}` : ''});`
}).join('\n')
}
${sp} let state = await ${initFn}(context, ${scenarioNameWithReplacements}, ['${tags.join("', '") || ''}'], [${examples?.map(s => tl(s)).join(',')}]);
${renderedSteps}
${sp} await afterScenario(state);
${sp} }
${sp});
Expand All @@ -198,7 +191,7 @@ ${sp});

return `
${sp}test${attrs}('${q(child.scenario!.keyword)}: ${q(child.scenario!.name)}${tagTextForVitest}', async (context) => {
${sp} let state = await ${initFn}(context, '${q(child.scenario!.name)}', ['${tags.join("', '") || ''}'], [${steps?.map(s => '`'+s+'`').join(',')}]);
${sp} let state = await ${initFn}(context, '${q(child.scenario!.name)}', ['${tags.join("', '") || ''}'], [${child.scenario?.steps.map(s => tl(s.text)).join(',')}]);
${renderSteps(child.scenario!.steps as Step[], config, sp + ' ', isExploded ? `${explodedIdx+1}` : '')}
${sp} await afterScenario(state);
${sp}});
Expand All @@ -214,14 +207,14 @@ function renderSteps(steps:Step[], config:QuickPickleConfig, sp = ' ', exploded
let data = JSON.stringify(step.dataTable.rows.map(r => {
return r.cells.map(c => c.value)
}))
return `${sp}await gherkinStep('${q(step.text)}', state, ${step.location.line}, ${minus}${idx+1}, ${explodedText || 'undefined'}, ${data});`
return `${sp}await gherkinStep(${tl(step.text)}, state, ${step.location.line}, ${minus}${idx+1}, ${explodedText || 'undefined'}, ${data});`
}
else if (step.docString) {
let data = JSON.stringify(pick(step.docString, ['content','mediaType']))
return `${sp}await gherkinStep('${q(step.text)}', state, ${step.location.line}, ${minus}${idx+1}, ${explodedText || 'undefined'}, ${data});`
return `${sp}await gherkinStep(${tl(step.text)}, state, ${step.location.line}, ${minus}${idx+1}, ${explodedText || 'undefined'}, ${data});`
}

return `${sp}await gherkinStep('${q(step.text)}', state, ${step.location.line}, ${minus}${idx+1}${explodedText ? `, ${explodedText}` : ''});`
return `${sp}await gherkinStep(${tl(step.text)}, state, ${step.location.line}, ${minus}${idx+1}${explodedText ? `, ${explodedText}` : ''});`
}).join('\n')
}

Expand All @@ -230,7 +223,27 @@ function renderSteps(steps:Step[], config:QuickPickleConfig, sp = ' ', exploded
* @param t string
* @returns string
*/
const q = (t:string) => (t.replace(/'/g, "\\'"))
const q = (t:string) => (t.replace(/\\/g,'\\\\').replace(/'/g, "\\'"))

/**
* Escapes text and returns a properly escaped template literal,
* since steps must be rendered in this way for Scenario Outlines
*
* For example:
* tl('escaped text') returns '`escaped text`'
*
* @param text string
* @returns string
*/
const tl = (text:string) => {
// Step 1: Escape existing escape sequences (e.g., \`)
text = text.replace(/\\/g, '\\\\');
// Step 2: Escape backticks
text = text.replace(/`/g, '\\`');
// Step 3: Escape $ if followed by { and not already escaped
text = text.replace(/\$\{(?!_\d+\})/g, '\\$\\{');
return '`' + text + '`';
}

/**
* Creates a 2d array of all possible combinations of the items in the input array
Expand Down
4 changes: 2 additions & 2 deletions packages/main/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ Feature: Exploding Tags
let output2 = await plugin.transform(feature2, 'test.feature')
test('exploding tags work as expected', () => {
expect(output2).toMatch(/test\.concurrent[\s\S]+?test\.concurrent/m)
expect(output2).toMatch(/I run the tests', state, 5, 1, 1\)/m)
expect(output2).toMatch(/I run the tests', state, 5, 1, 2\)/m)
expect(output2).toMatch(/I run the tests`, state, 5, 1, 1\)/m)
expect(output2).toMatch(/I run the tests`, state, 5, 1, 2\)/m)
expect(output2).not.toMatch(/test\.concurrent[\s\S]+?test\.concurrent[/s/S]+?test\.concurrent/m)
})

Expand Down
39 changes: 39 additions & 0 deletions packages/main/tests/test.feature
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,42 @@ Feature: Basic Test
Example: This is a failing test
Given I run the tests
Then the tests should fail

Rule: Tests must have appropriate guards against escaping
# | character 14 - 13 = 1 character 113 - 13 = 100 |
Example: `someone` is ${sneaky} \${with} \\${backslashes} \\\${and} \$\{other} \`things\\` 'like' \'quotes\\'
When I set the data variable "escapedDoubleQuote" to "\""
And I set the data variable "escapedSingleQuote" to "\'"
And I set the data variable "escapedBacktick" to "\`"
And I set the data variable "doubleEscapedDoubleQuote" to "\\'"
And I set the data variable "doubleEscapedSingleQuote" to "\\'"
And I set the data variable "doubleEscapedBacktick" to "\\`"
And I set the data variable "unescapedError" to "${throw new Error(muuahahaha!)}"
And I set the data variable "escapedError" to "\${throw new Error(muuahahaha!)}"
Then the variable "data.escapedDoubleQuote" should be 1 character long
And the variable "data.escapedSingleQuote" should be 1 character long
And the variable "data.escapedBacktick" should be 2 characters long
And the variable "data.doubleEscapedDoubleQuote" should be 2 character long
And the variable "data.doubleEscapedSingleQuote" should be 2 character long
And the variable "data.doubleEscapedBacktick" should be 3 characters long
And the variable "data.unescapedError" should be 31 characters long
And the variable "data.escapedError" should be 32 characters long
And the variable "info.scenario" should be 100 characters long

Example: Outside a {string} variable, escaped quotes are two characters, but the file is still safe
When I raw set the data variable "escapedDoubleQuote" to \"
And I raw set the data variable "escapedSingleQuote" to \'
And I raw set the data variable "escapedBacktick" to \`
And I raw set the data variable "doubleEscapedDoubleQuote" to \\"
And I raw set the data variable "doubleEscapedSingleQuote" to \\'
And I raw set the data variable "doubleEscapedBacktick" to \\`
And I raw set the data variable "unescapedError" to ${throw new Error(muuahahaha!)}
And I raw set the data variable "escapedError" to \${throw new Error(muuahahaha!)}
Then the variable "data.escapedDoubleQuote" should be 2 characters long
And the variable "data.escapedSingleQuote" should be 2 characters long
And the variable "data.escapedBacktick" should be 2 characters long
And the variable "data.doubleEscapedDoubleQuote" should be 3 character long
And the variable "data.doubleEscapedSingleQuote" should be 3 character long
And the variable "data.doubleEscapedBacktick" should be 3 characters long
And the variable "data.unescapedError" should be 31 characters long
And the variable "data.escapedError" should be 32 characters long
14 changes: 13 additions & 1 deletion packages/main/tests/tests.steps.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect } from "vitest";
import { Given, Then, When } from "../src";
import type { DataTable } from "@cucumber/cucumber";
import { clone, get } from "lodash-es";
import { clone, get, set } from "lodash-es";
import type { DocString } from "../src/models/DocString";

Given("I run the tests", () => {});
Expand Down Expand Up @@ -39,13 +39,25 @@ Then('the sum should be {int}', (world, int) => {
expect(world.numbers.reduce((a,b) => a + b, 0)).toBe(int)
})

When('I set the data variable/value/property {string} to {string}', (world, prop, value) => {
if (!world.data) world.data = {}
set(world.data, prop, value)
})
When('I raw set the data variable/value/property {string} to {}', (world, prop, value) => {
if (!world.data) world.data = {}
set(world.data, prop, value)
})
Then('the variable/value/property/typeof {string} should include/contain/equal/match/be {string}', (world, prop, expected) => {
let value = get(world,prop)
let testValue = world.info.step.match(/^the typeof/) ? typeof value : value

if (world.info.step.match(/" should (?:equal|match|be)"/)) expect(testValue?.toString() ?? '').toBe(expected)
else expect(testValue?.toString() ?? '').toContain(expected)
})
Then('the variable/value/property {string} should be {int} character(s) long', (world, prop, length) => {
let value = get(world,prop)
expect(value?.toString()?.length).toBe(parseInt(length))
})

Then('the stack for error {int} should contain {string}', async (world, idx, expected) => {
let stack = world.info.errors[idx-1].stack
Expand Down

0 comments on commit 2bb4509

Please sign in to comment.