-
-
Notifications
You must be signed in to change notification settings - Fork 68
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Python enable markdown acceptance tests #64
base: main
Are you sure you want to change the base?
Changes from all commits
e5dad4c
2085773
709372c
b8ce724
c9c88a1
0ef264e
a697916
6387c51
bbe4d21
718d07c
e6e1269
4fb5930
12a3a93
c0b9467
16fbb63
7f1790c
6204f30
18b1cf0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -130,7 +130,13 @@ export default class GherkinInMarkdownTokenMatcher implements ITokenMatcher<Toke | |
let result = false | ||
if (token.line.startsWith('|')) { | ||
const tableCells = token.line.getTableCells() | ||
if (this.isGfmTableSeparator(tableCells)) result = true | ||
if (this.isGfmTableSeparator(tableCells)) { | ||
// Maintain consistency with Python implementation | ||
token.matchedType = TokenType.Comment | ||
token.matchedText = undefined | ||
token.matchedIndent=0 | ||
result = true | ||
Comment on lines
+135
to
+138
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added these in an attempt to make the Python and Javascript versions consistent. But the Javascript acceptance tests currently fail because of my comment above regarding If I try to remove the Guidance here would be appreciated. |
||
} | ||
} | ||
return this.setTokenMatched(token, null, result) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,35 +34,58 @@ describe('GherkinInMarkdownTokenMatcher', function () { | |
assert.strictEqual(token.matchedText, 'hello') | ||
}) | ||
|
||
it('matches FeatureLine without the Feature: keyword', () => { | ||
const line = new GherkinLine('# hello', location.line) | ||
const token = new Token(line, location) | ||
assert(tm.match_FeatureLine(token)) | ||
assert.strictEqual(token.matchedType, TokenType.FeatureLine) | ||
assert.strictEqual(token.matchedKeyword, undefined) | ||
assert.strictEqual(token.matchedText, '# hello') | ||
}) | ||
|
||
it('matches bullet Step', () => { | ||
const line = new GherkinLine(' * Given I have 3 cukes', location.line) | ||
const token = new Token(line, location) | ||
assert(tm.match_StepLine(token)) | ||
assert.strictEqual(token.matchedType, TokenType.StepLine) | ||
assert.strictEqual(token.matchedKeyword, 'Given ') | ||
assert.strictEqual(token.matchedKeywordType, 'Context') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added checks here since I had a bug when generating pickles. Is this an appropriate place to be checking |
||
assert.strictEqual(token.matchedText, 'I have 3 cukes') | ||
assert.strictEqual(token.location.column, 6) | ||
}) | ||
|
||
it('matches plus Step', () => { | ||
const line = new GherkinLine(' + Given I have 3 cukes', location.line) | ||
const token = new Token(line, location) | ||
assert(tm.match_StepLine(token)) | ||
assert.strictEqual(token.matchedType, TokenType.StepLine) | ||
assert.strictEqual(token.matchedKeyword, 'Given ') | ||
assert.strictEqual(token.matchedKeywordType, 'Context') | ||
assert.strictEqual(token.matchedText, 'I have 3 cukes') | ||
assert.strictEqual(token.location.column, 6) | ||
}) | ||
|
||
it('matches hyphen Step', () => { | ||
const line = new GherkinLine(' - Given I have 3 cukes', location.line) | ||
const token = new Token(line, location) | ||
assert(tm.match_StepLine(token)) | ||
assert.strictEqual(token.matchedType, TokenType.StepLine) | ||
assert.strictEqual(token.matchedKeyword, 'Given ') | ||
assert.strictEqual(token.matchedKeywordType, 'Context') | ||
assert.strictEqual(token.matchedText, 'I have 3 cukes') | ||
assert.strictEqual(token.location.column, 6) | ||
}) | ||
|
||
it('matches a when Step', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this test as I had a bug in my Python parser. Is this an appropriate location to be testing If these are too low level (white-box / implementation detail), then the tests should be moved/removed |
||
const line = new GherkinLine(' - When I do something', location.line) | ||
const token = new Token(line, location) | ||
assert(tm.match_StepLine(token)) | ||
assert.strictEqual(token.matchedType, TokenType.StepLine) | ||
assert.strictEqual(token.matchedKeyword, 'When ') | ||
assert.strictEqual(token.matchedKeywordType, 'Action') | ||
assert.strictEqual(token.matchedText, 'I do something') | ||
assert.strictEqual(token.location.column, 6) | ||
}) | ||
|
||
it('matches arbitrary text as Other', () => { | ||
const line = new GherkinLine('Whatever', location.line) | ||
|
@@ -186,4 +209,21 @@ describe('GherkinInMarkdownTokenMatcher', function () { | |
] | ||
assert.deepStrictEqual(t.matchedItems, expectedItems) | ||
}) | ||
|
||
it('matches arbitrary text as Empty after the FeatureLine has already been matched', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is testing implementation detail, but I find it useful documentation for the token matcher - there is inherent state management here that I found confusing to begin with (resulting in a bug in my Python translation) This behaviour is captured under the acceptance tests, but was useful for me for understanding where differences between implementations were. Thoughts - is this useful to keep, or preferable to remove? |
||
// White Box testing - implementation detail... | ||
// Given the FeatureLine has already been matched | ||
const tFeatureLine = new Token(new GherkinLine('# something arbitrary', location.line), location); | ||
assert(tm.match_FeatureLine(tFeatureLine)) | ||
|
||
|
||
const t = new Token(new GherkinLine('arbitrary text', location.line), location); | ||
// (tm as any).matchedFeatureLine = true | ||
assert(tm.match_Empty(t)) | ||
assert.strictEqual(t.matchedType, TokenType.Empty) | ||
const expectedItems: Item[] =undefined | ||
assert.deepStrictEqual(t.matchedItems, expectedItems) | ||
assert.strictEqual(t.matchedKeyword, undefined) | ||
assert.strictEqual(t.matchedText, undefined) | ||
} ) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,16 +6,17 @@ import GherkinClassicTokenMatcher from '../src/GherkinClassicTokenMatcher' | |
import AstNode from '../src/AstNode' | ||
import generateMessages from '../src/generateMessages' | ||
import GherkinInMarkdownTokenMatcher from '../src/GherkinInMarkdownTokenMatcher' | ||
import { StepKeywordType } from '@cucumber/messages' | ||
|
||
describe('Parser', function () { | ||
describe('with Gherkin Classic', () => { | ||
let parser: Parser<AstNode> | ||
beforeEach( | ||
() => | ||
(parser = new Parser<AstNode>( | ||
new AstBuilder(messages.IdGenerator.incrementing()), | ||
new GherkinClassicTokenMatcher() | ||
)) | ||
(parser = new Parser<AstNode>( | ||
new AstBuilder(messages.IdGenerator.incrementing()), | ||
new GherkinClassicTokenMatcher() | ||
)) | ||
) | ||
|
||
it('parses a simple feature', function () { | ||
|
@@ -93,20 +94,20 @@ describe('Parser', function () { | |
try { | ||
parser.parse( | ||
'# a comment\n' + | ||
'Feature: Foo\n' + | ||
' Scenario: Bar\n' + | ||
' Given x\n' + | ||
' ```\n' + | ||
' unclosed docstring\n' | ||
'Feature: Foo\n' + | ||
' Scenario: Bar\n' + | ||
' Given x\n' + | ||
' ```\n' + | ||
' unclosed docstring\n' | ||
) | ||
} catch (expected) { | ||
ast = parser.parse( | ||
'Feature: Foo\n' + | ||
' Scenario: Bar\n' + | ||
' Given x\n' + | ||
' """\n' + | ||
' closed docstring\n' + | ||
' """' | ||
' Scenario: Bar\n' + | ||
' Given x\n' + | ||
' """\n' + | ||
' closed docstring\n' + | ||
' """' | ||
) | ||
} | ||
|
||
|
@@ -155,11 +156,11 @@ describe('Parser', function () { | |
it('interpolates data tables', function () { | ||
const envelopes = generateMessages( | ||
'Feature: Foo\n' + | ||
' Scenario Outline: Parenthesis\n' + | ||
' Given the thing <is (not) triggered> and has <value>\n' + | ||
' Examples:\n' + | ||
' | is (not) triggered | value |\n' + | ||
' | is triggered | foo |\n ', | ||
' Scenario Outline: Parenthesis\n' + | ||
' Given the thing <is (not) triggered> and has <value>\n' + | ||
' Examples:\n' + | ||
' | is (not) triggered | value |\n' + | ||
' | is triggered | foo |\n ', | ||
'', | ||
messages.SourceMediaType.TEXT_X_CUCUMBER_GHERKIN_PLAIN, | ||
{ includePickles: true, newId: messages.IdGenerator.incrementing() } | ||
|
@@ -194,10 +195,10 @@ describe('Parser', function () { | |
let parser: Parser<AstNode> | ||
beforeEach( | ||
() => | ||
(parser = new Parser<AstNode>( | ||
new AstBuilder(messages.IdGenerator.incrementing()), | ||
new GherkinInMarkdownTokenMatcher() | ||
)) | ||
(parser = new Parser<AstNode>( | ||
new AstBuilder(messages.IdGenerator.incrementing()), | ||
new GherkinInMarkdownTokenMatcher() | ||
)) | ||
) | ||
|
||
it('does not parse a feature description', function () { | ||
|
@@ -308,5 +309,128 @@ description | |
|
||
assert.strictEqual(pickle.steps[0].argument.docString.content, '```what') | ||
}) | ||
|
||
it("parses Markdown data tables with headers", () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a re-implementation of the |
||
const markdown = `## Feature: DataTables | ||
|
||
### Scenario: minimalistic | ||
|
||
* Given a simple data table | ||
| foo | bar | | ||
| --- | --- | | ||
| boz | boo | | ||
` | ||
const ast = parser.parse(markdown) | ||
const gherkinDocument: messages.GherkinDocument = { | ||
"comments": [ | ||
{ | ||
"location": { | ||
"column": 3, | ||
"line": 7 | ||
}, | ||
"text": undefined | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This attribute is required, but conflicts with the Python implementation (see below) |
||
} | ||
], | ||
"feature": { | ||
"children": [ | ||
{ | ||
"scenario": { | ||
"description": "", | ||
"examples": [], | ||
"id": "3", | ||
"keyword": "Scenario", | ||
"location": { | ||
"column": 5, | ||
"line": 3 | ||
}, | ||
"name": "minimalistic", | ||
"steps": [ | ||
{ | ||
"dataTable": { | ||
"location": { | ||
"column": 3, | ||
"line": 6 | ||
}, | ||
"rows": [ | ||
{ | ||
"cells": [ | ||
{ | ||
"location": { | ||
"column": 5, | ||
"line": 6 | ||
}, | ||
"value": "foo" | ||
}, | ||
{ | ||
"location": { | ||
"column": 11, | ||
"line": 6 | ||
}, | ||
"value": "bar" | ||
} | ||
], | ||
"id": "0", | ||
"location": { | ||
"column": 3, | ||
"line": 6 | ||
} | ||
}, | ||
{ | ||
"cells": [ | ||
{ | ||
"location": { | ||
"column": 5, | ||
"line": 8 | ||
}, | ||
"value": "boz" | ||
}, | ||
{ | ||
"location": { | ||
"column": 11, | ||
"line": 8 | ||
}, | ||
"value": "boo" | ||
} | ||
], | ||
"id": "1", | ||
"location": { | ||
"column": 3, | ||
"line": 8 | ||
} | ||
} | ||
] | ||
}, | ||
"id": "2", | ||
docString: undefined, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this attribute is required in the Javascript implementation, but not in the Python implementation. |
||
"keyword": "Given ", | ||
"keywordType": StepKeywordType.CONTEXT, | ||
"location": { | ||
"column": 3, | ||
"line": 5 | ||
}, | ||
"text": "a simple data table" | ||
} | ||
], | ||
"tags": [] | ||
} | ||
} | ||
], | ||
"description": "", | ||
"keyword": "Feature", | ||
"language": "en", | ||
"location": { | ||
"column": 4, | ||
"line": 1 | ||
}, | ||
"name": "DataTables", | ||
"tags": [] | ||
} | ||
} | ||
assert.deepStrictEqual(ast, gherkinDocument) | ||
}) | ||
|
||
}) | ||
|
||
|
||
}) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,12 +7,18 @@ | |
from gherkin.token_scanner import TokenScanner | ||
from gherkin.token_formatter_builder import TokenFormatterBuilder | ||
from gherkin.parser import Parser | ||
from gherkin.token_matcher_markdown import GherkinInMarkdownTokenMatcher | ||
|
||
files = sys.argv[1:] | ||
if sys.version_info < (3, 0) and os.name != 'nt': # for Python2 unless on Windows native | ||
UTF8Writer = codecs.getwriter('utf8') | ||
sys.stdout = UTF8Writer(sys.stdout) | ||
|
||
parser = Parser(TokenFormatterBuilder()) | ||
for file in files: | ||
scanner = TokenScanner(file) | ||
print(parser.parse(scanner)) | ||
|
||
if(file.endswith('.md')): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This logic is now spread across a few files (see Is there a better way to encapsulate this behaviour? Caveat from
|
||
print(parser.parse(scanner, GherkinInMarkdownTokenMatcher()) ) | ||
else: | ||
print(parser.parse(scanner)) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
from gherkin.pickles.compiler import Compiler | ||
from gherkin.errors import ParserError, CompositeParserException | ||
from gherkin.stream.id_generator import IdGenerator | ||
from gherkin.token_matcher_markdown import GherkinInMarkdownTokenMatcher | ||
|
||
def create_errors(errors, uri): | ||
for error in errors: | ||
|
@@ -28,7 +29,10 @@ def enum(self, source_event): | |
source = source_event['source']['data'] | ||
|
||
try: | ||
gherkin_document = self.parser.parse(source) | ||
matcher=None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comment above - DRY |
||
if(uri.endswith('.md')): | ||
matcher=GherkinInMarkdownTokenMatcher() | ||
gherkin_document = self.parser.parse(source, matcher) | ||
gherkin_document['uri'] = uri | ||
|
||
if (self.options.print_source): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,18 @@ def source_event(path): | |
'source': { | ||
'uri': path, | ||
'data': io.open(path, 'r', encoding='utf8', newline='').read(), | ||
'mediaType': 'text/x.cucumber.gherkin+plain' | ||
'mediaType': _media_type(path) | ||
} | ||
} | ||
return event | ||
|
||
|
||
def _media_type(path): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comment above - DRY |
||
if(path.endswith(".feature")): | ||
return 'text/x.cucumber.gherkin+plain' | ||
if(path.endswith(".feature.md")): | ||
return 'text/x.cucumber.gherkin+markdown' | ||
|
||
class SourceEvents: | ||
def __init__(self, paths): | ||
self.paths = paths | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mpkorstanje is this still correct with the package changes?
Looking at the changelog, it looks to have been moved here
That is different from the python implementation and now potentially causes a dependency issue? I think there's a conflict between the javascript and python implementations with minor differences between the ASTs - which I'm not sure how to resolve.